Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

fscache: load directories only once #205

Merged
merged 1 commit into from
Jun 25, 2014
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 56 additions & 9 deletions compat/win32/fscache.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ struct fsentry {
union {
/* Reference count of the directory listing. */
volatile long refcnt;
/* Handle to wait on the loading thread. */
HANDLE hwait;
struct {
/* More stat members (only used for file entries). */
off64_t st_size;
Expand Down Expand Up @@ -260,24 +262,51 @@ static inline int fscache_enabled(const char *path)
return enabled > 0 && !is_absolute_path(path);
}

/*
* Looks up a cache entry, waits if its being loaded by another thread.
* The mutex must be owned by the calling thread.
*/
static struct fsentry *fscache_get_wait(struct fsentry *key)
{
struct fsentry *fse = hashmap_get(&map, key, NULL);

/* return if its a 'real' entry (future entries have refcnt == 0) */
if (!fse || fse->list || fse->refcnt)
return fse;

/* create an event and link our key to the future entry */
key->hwait = CreateEvent(NULL, TRUE, FALSE, NULL);
key->next = fse->next;
fse->next = key;

/* wait for the loading thread to signal us */
LeaveCriticalSection(&mutex);
WaitForSingleObject(key->hwait, INFINITE);
CloseHandle(key->hwait);
EnterCriticalSection(&mutex);

/* repeat cache lookup */
return hashmap_get(&map, key, NULL);
}

/*
* Looks up or creates a cache entry for the specified key.
*/
static struct fsentry *fscache_get(struct fsentry *key)
{
struct fsentry *fse;
struct fsentry *fse, *future, *waiter;

EnterCriticalSection(&mutex);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. From http://msdn.microsoft.com/en-us/library/windows/desktop/ms682608%28v=vs.85%29.aspx I expected that the second thread to hit the same code path would have to wait until the first one added the entry, and therefore would not add another entry... (sorry if I am stupidly missing anything)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actual loading is done outside the critical section (see lines 290/325 below). We don't want to block concurrent readers while doing expensive IO.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad for missing this! Thanks for the education!

/* check if entry is in cache */
fse = hashmap_get(&map, key, NULL);
fse = fscache_get_wait(key);
if (fse) {
fsentry_addref(fse);
LeaveCriticalSection(&mutex);
return fse;
}
/* if looking for a file, check if directory listing is in cache */
if (!fse && key->list) {
fse = hashmap_get(&map, key->list, NULL);
fse = fscache_get_wait(key->list);
if (fse) {
LeaveCriticalSection(&mutex);
/* dir entry without file entry -> file doesn't exist */
Expand All @@ -286,16 +315,34 @@ static struct fsentry *fscache_get(struct fsentry *key)
}
}

/* add future entry to indicate that we're loading it */
future = key->list ? key->list : key;
future->next = NULL;
future->refcnt = 0;
hashmap_add(&map, future);

/* create the directory listing (outside mutex!) */
LeaveCriticalSection(&mutex);
fse = fsentry_create_list(key->list ? key->list : key);
if (!fse)
fse = fsentry_create_list(future);
EnterCriticalSection(&mutex);

/* remove future entry and signal waiting threads */
hashmap_remove(&map, future, NULL);
waiter = future->next;
while (waiter) {
HANDLE h = waiter->hwait;
waiter = waiter->next;
SetEvent(h);
}

/* leave on error (errno set by fsentry_create_list) */
if (!fse) {
LeaveCriticalSection(&mutex);
return NULL;
}

EnterCriticalSection(&mutex);
/* add directory listing if it hasn't been added by some other thread */
if (!hashmap_get(&map, key, NULL))
fscache_add(fse);
/* add directory listing to the cache */
fscache_add(fse);

/* lookup file entry if requested (fse already points to directory) */
if (key->list)
Expand Down