From da1622ca0651b3343f0df598c1ab469eed53fc28 Mon Sep 17 00:00:00 2001 From: janwas Date: Wed, 2 Mar 2005 12:06:40 +0000 Subject: [PATCH] reloading is optimized and armor-plated. eliminated an evil race condition. wdir_watch.cpp: remove debug output and fix dir_get_changed_file interface bug (was returning ERR_AGAIN instead of 1) This was SVN commit r1964. --- source/lib/res/res.cpp | 98 +++++++++++++++++++++------- source/lib/sysdep/win/wdir_watch.cpp | 17 ++--- 2 files changed, 80 insertions(+), 35 deletions(-) diff --git a/source/lib/res/res.cpp b/source/lib/res/res.cpp index 1961a1e990..8edcb6ddaa 100755 --- a/source/lib/res/res.cpp +++ b/source/lib/res/res.cpp @@ -10,18 +10,28 @@ #include -// called from res_reload_changed_files and via console. +// called by res_reload and res_reload_changed_files (which will already +// have rebuilt the VFS - doing so more than once a frame is unnecessary). +static int reload_without_rebuild(const char* fn) +{ + // invalidate this file's cached blocks to make sure its contents are + // loaded anew. + CHECK_ERR(file_invalidate_cache(fn)); + + CHECK_ERR(h_reload(fn)); + + return 0; +} + + +// called via console command. int res_reload(const char* fn) { // if currently maps to an archive, the VFS must switch // over to using the loose file (that was presumably changed). - vfs_rebuild(); + CHECK_ERR(vfs_rebuild()); - // invalidate this file's cached blocks to make sure its contents are - // loaded anew. - file_invalidate_cache(fn); - - return h_reload(fn); + return reload_without_rebuild(fn); } @@ -44,36 +54,80 @@ int res_cancel_watch(const intptr_t watch) // than asynchronous notifications: everything would need to be thread-safe. int res_reload_changed_files() { - char n_path[PATH_MAX]; - while(dir_get_changed_file(n_path) == 0) + // array of reloads requested this frame (see 'do we really need to + // reload' below). go through gyrations to avoid heap allocs. + const size_t MAX_RELOADS_PER_FRAME = 12; + typedef char Path[VFS_MAX_PATH]; + typedef Path PathList[MAX_RELOADS_PER_FRAME]; + PathList pending_reloads; + + uint num_pending = 0; + // process only as many notifications as we have room for; the others + // will be handled next frame. it's not imagineable that they'll pile up. + while(num_pending < MAX_RELOADS_PER_FRAME) { + // get next notification + char n_path[PATH_MAX]; + int ret = dir_get_changed_file(n_path); + CHECK_ERR(ret); // error? (doesn't cover 'none available') + if(ret != 0) // none available; done. + break; + // convert to VFS path char p_path[PATH_MAX]; CHECK_ERR(file_make_full_portable_path(n_path, p_path)); - char vfs_path[VFS_MAX_PATH]; + char* vfs_path = pending_reloads[num_pending]; CHECK_ERR(vfs_make_vfs_path(p_path, vfs_path)); - // various early-out checks that reduce debug output clutter, - // avoid the overhead of searching through all Handles, and - // try to eliminate repeated reloads: + // do we really need to reload? try to avoid the considerable cost of + // rebuilding VFS and scanning all Handles. + // + // note: be careful to avoid 'race conditions' depending on the + // timeframe in which notifications reach us. + // example: editor deletes a.tga; we are notified; reload is + // triggered but fails since the file isn't found; further + // notifications (e.g. renamed a.tmp to a.tga) come within x [ms] and + // are ignored due to a time limit. + // therefore, we can only check for multiple reload requests a frame; + // to that purpose, an array is built and duplicates ignored. const char* ext = strrchr(vfs_path, '.'); - // .. ignore directory change notifications, because we get - // per-file notifications anyway. (note: assume no extension => - // it's a directory). this also protects the strcmp calls below. + // .. directory change notification; ignore because we get + // per-file notifications anyway. (note: assume no extension => + // it's a directory). this also protects the strcmp calls below. if(!ext) continue; - // .. ignore files that can't be reloaded anyway. + // .. compiled XML files the engine writes out by the hundreds; + // skipping them is a big performance gain. if(!strcmp(ext, ".xmb")) continue; - // .. skip temp files, because many apps save by creating a temp - // file, deleting the original, and renaming the temp file. - // => avoids 2 redundant reloads. + // .. temp files, usually created when an editor saves a file + // (delete, create temp, rename temp); no need to reload those. if(!strcmp(ext, ".tmp")) continue; + // .. more than one notification for a file; only reload once. + // note: this doesn't suffer from the 'reloaded too early' + // problem described above; if there's more than one + // request in the array, the file has since been written. + for(uint i = 0; i < num_pending; i++) + if(!strcmp(pending_reloads[i], vfs_path)) + continue; - int ret = res_reload(vfs_path); - assert(ret == 0); + // path has already been written to pending_reloads, + // so just mark it valid. + num_pending++; } + // rebuild VFS, in case a file that has been changed is currently + // mounted from an archive (reloading would just grab the unchanged + // version in the archive). the rebuild sees differing mtimes and + // always choses the loose file version. only do this once + // (instead of per reload request) because it's slow (> 1s)! + if(num_pending != 0) + CHECK_ERR(vfs_rebuild()); + + // now actually reload all files in the array we built + for(uint i = 0; i < num_pending; i++) + CHECK_ERR(reload_without_rebuild(pending_reloads[i])); + return 0; } diff --git a/source/lib/sysdep/win/wdir_watch.cpp b/source/lib/sysdep/win/wdir_watch.cpp index 35e769c5b0..9885571115 100755 --- a/source/lib/sysdep/win/wdir_watch.cpp +++ b/source/lib/sysdep/win/wdir_watch.cpp @@ -321,17 +321,6 @@ static int extract_events(Watch* w) for(int i = 0; i < (int)fni->FileNameLength/2; i++) fn += (char)fni->FileName[i]; - const char* action; - switch(fni->Action) - { - case FILE_ACTION_ADDED: action = "FILE_ACTION_ADDED"; break; - case FILE_ACTION_REMOVED: action = "FILE_ACTION_REMOVED"; break; - case FILE_ACTION_MODIFIED: action = "FILE_ACTION_MODIFIED"; break; - case FILE_ACTION_RENAMED_OLD_NAME: action = "FILE_ACTION_RENAMED_OLD_NAME"; break; - case FILE_ACTION_RENAMED_NEW_NAME: action = "FILE_ACTION_RENAMED_NEW_NAME"; break; - } - debug_out("PACKET %s %s\n", fn.c_str(), action); - pending_events.push_back(fn); // advance to next entry in buffer (variable length) @@ -386,7 +375,9 @@ static int get_packet() } -// fn must hold at least PATH_MAX chars. +// if a file change notification is pending, store its filename in and +// return 0; otherwise, return 1 ('none currently pending') or an error code. +// must hold at least PATH_MAX chars. int dir_get_changed_file(char* fn) { // queue one or more events, or return 1 if none pending. @@ -394,7 +385,7 @@ int dir_get_changed_file(char* fn) // nothing to return; call again later. if(pending_events.empty()) - return ERR_AGAIN; + return 1; const std::string& fn_s = pending_events.front(); strcpy_s(fn, PATH_MAX, fn_s.c_str());