Skip to content

Commit

Permalink
Single-threaded db init (5354)
Browse files Browse the repository at this point in the history
On examination, I think there's no real reason for this to be
multithreaded in the first place.  Relative to des regeneration (which
isn't threaded), db regeneration/loading is quite fast, and I didn't see
a major time savings either way even for just db initialization.

For the record, I'm 80% sure that the sporadic db init bugs that led to
2e17027 and d00fd6f were caused by the following: crawl would
spawn multiple threads even if sqlite was configured or if databases
were opened in single threading mode, in which case there was of course
no thread safety.  The code that opened the database was also entirely
implicit about which threading mode to use.  By forcing single-threading
mode in sqlite I could replicate the exact reported behavior (random
failures to build the db about 20% of the time) on my OS X build, so I
suspect that the problem may have been caused by people accidentally
linking against a sqlite version that had single threading by default.

If anyone ever wants to resurrect a multi-threaded version of this, I
recommend explicitly doing something like:
`sqlite3_config(SQLITE_CONFIG_SERIALIZED);` before spawning any threads.
There's an equivalent parameter you can pass when opening a database, as
well.
  • Loading branch information
rawlins committed Jul 11, 2017
1 parent d00fd6f commit 6841d05
Showing 1 changed file with 1 addition and 29 deletions.
30 changes: 1 addition & 29 deletions crawl-ref/source/database.cc
Expand Up @@ -331,40 +331,12 @@ void TextDB::_regenerate_db()
// DB system
// ----------------------------------------------------------------------

#if !defined(DGAMELAUNCH) && !defined(TARGET_OS_WINDOWS) && !defined(TARGET_OS_LINUX)
static void* init_db(void *arg)
{
AllDBs[(intptr_t)arg].init();
return 0;
}
#endif

#define NUM_DB ARRAYSZ(AllDBs)

void databaseSystemInit()
{
// Note: if you're building contrib libraries initially checked out
// before 2011-12-28 and this assertion fails, please make sure you have
// the current version ("git submodule sync;git submodule update --init").
ASSERT(sqlite3_threadsafe());

thread_t th[NUM_DB];
for (unsigned int i = 0; i < NUM_DB; i++)
// Using threads for loading on Windows at the moment seems to cause
// random failures to find files (#5354); thus disabling it here until
// we can identify what's going on.
// 2017: this is also happening (with low frequency) on linux builds.
#if !defined(DGAMELAUNCH) && !defined(TARGET_OS_WINDOWS) && !defined(TARGET_OS_LINUX)
if (thread_create_joinable(&th[i], init_db, (void*)(intptr_t)i))
#endif
{
// if thread creation fails, do it serially
th[i] = 0;
AllDBs[i].init();
}
for (unsigned int i = 0; i < NUM_DB; i++)
if (th[i])
thread_join(th[i]);
AllDBs[i].init();
}

void databaseSystemShutdown()
Expand Down

0 comments on commit 6841d05

Please sign in to comment.