Bug 1603801 [patch] Avoid dcache pollution from sdb_measureAccess() r=mt
As implemented, when sdb_measureAccess() runs it creates up to 10,000 negative
dcache entries (cached nonexistent filenames).
There is no advantage to leaving these particular filenames in the cache; they
will never be searched again. Subsequent runs will run a new test with an
intentionally different set of filenames. This can have detrimental effects on
some systems; a massive negative dcache can lead to memory or performance
problems.
Since not all platforms have a problem with negative dcache entries, this patch
is limitted to those platforms that request it at compilie time (Linux is
current the only patch that does.)
Differential Revision:
https://phabricator.services.mozilla.com/D59652
--- a/coreconf/Linux.mk
+++ b/coreconf/Linux.mk
@@ -16,17 +16,17 @@ ifneq ($(OS_TARGET),Android)
USE_PTHREADS = 1
endif
ifeq ($(USE_PTHREADS),1)
IMPL_STRATEGY = _PTH
endif
DEFAULT_COMPILER = gcc
-DEFINES += -D_DEFAULT_SOURCE -D_BSD_SOURCE -D_POSIX_SOURCE
+DEFINES += -D_DEFAULT_SOURCE -D_BSD_SOURCE -D_POSIX_SOURCE -DSQL_MEASURE_USE_TEMP_DIR
ifeq ($(OS_TARGET),Android)
ifndef ANDROID_NDK
$(error Must set ANDROID_NDK to the path to the android NDK first)
endif
ifndef ANDROID_TOOLCHAIN_VERSION
$(error Must set ANDROID_TOOLCHAIN_VERSION to the requested version number)
endif
--- a/coreconf/config.gypi
+++ b/coreconf/config.gypi
@@ -357,16 +357,17 @@
[ 'OS=="linux" or OS=="android"', {
'defines': [
'LINUX2_1',
'LINUX',
'linux',
'_DEFAULT_SOURCE', # for <endian.h> functions, strdup, realpath, and getentropy
'_BSD_SOURCE', # for the above in glibc <= 2.19
'_POSIX_SOURCE', # for <signal.h>
+ 'SQL_MEASURE_USE_TEMP_DIR', # use tmpdir for the access calls
],
}],
[ 'OS=="dragonfly" or OS=="freebsd"', {
'defines': [
'FREEBSD',
],
}],
[ 'OS=="netbsd"', {
--- a/lib/softoken/sdb.c
+++ b/lib/softoken/sdb.c
@@ -395,46 +395,73 @@ static PRUint32
sdb_measureAccess(const char *directory)
{
PRUint32 i;
PRIntervalTime time;
PRIntervalTime delta;
PRIntervalTime duration = PR_MillisecondsToInterval(33);
const char *doesntExistName = "_dOeSnotExist_.db";
char *temp, *tempStartOfFilename;
- size_t maxTempLen, maxFileNameLen, directoryLength;
-
+ size_t maxTempLen, maxFileNameLen, directoryLength, tmpdirLength = 0;
+#ifdef SDB_MEASURE_USE_TEMP_DIR
+ /*
+ * on some OS's and Filesystems, creating a bunch of files and deleting
+ * them messes up the systems's caching, but if we create the files in
+ * a temp directory which we later delete, then the cache gets cleared
+ * up. This code uses several OS dependent calls, and it's not clear
+ * that temp directory use won't mess up other filesystems and OS caching,
+ * so if you need this for your OS, you can turn on the
+ * 'SDB_MEASURE_USE_TEMP_DIR' define in coreconf
+ */
+ const char template[] = "dbTemp.XXXXXX";
+ tmpdirLength = sizeof(template);
+#endif
/* no directory, just return one */
if (directory == NULL) {
return 1;
}
/* our calculation assumes time is a 4 bytes == 32 bit integer */
PORT_Assert(sizeof(time) == 4);
directoryLength = strlen(directory);
- maxTempLen = directoryLength + strlen(doesntExistName) + 1 /* potential additional separator char */
- + 11 /* max chars for 32 bit int plus potential sign */
- + 1; /* zero terminator */
+ maxTempLen = directoryLength + 1 /* dirname + / */
+ + tmpdirLength /* tmpdirname includes / */
+ + strlen(doesntExistName) /* filename base */
+ + 11 /* max chars for 32 bit int plus potential sign */
+ + 1; /* zero terminator */
- temp = PORT_Alloc(maxTempLen);
+ temp = PORT_ZAlloc(maxTempLen);
if (!temp) {
return 1;
}
/* We'll copy directory into temp just once, then ensure it ends
- * with the directory separator, then remember the position after
- * the separator, and calculate the number of remaining bytes. */
+ * with the directory separator. */
strcpy(temp, directory);
if (directory[directoryLength - 1] != PR_GetDirectorySeparator()) {
temp[directoryLength++] = PR_GetDirectorySeparator();
}
- tempStartOfFilename = temp + directoryLength;
+
+#ifdef SDB_MEASURE_USE_TEMP_DIR
+ /* add the template for a temporary subdir, and create it */
+ strcat(temp, template);
+ if (!mkdtemp(temp)) {
+ PORT_Free(temp);
+ return 1;
+ }
+ /* and terminate that tmp subdir with a / */
+ strcat(temp, "/");
+#endif
+
+ /* Remember the position after the last separator, and calculate the
+ * number of remaining bytes. */
+ tempStartOfFilename = temp + directoryLength + tmpdirLength;
maxFileNameLen = maxTempLen - directoryLength;
/* measure number of Access operations that can be done in 33 milliseconds
* (1/30'th of a second), or 10000 operations, which ever comes first.
*/
time = PR_IntervalNow();
for (i = 0; i < 10000u; i++) {
PRIntervalTime next;
@@ -448,16 +475,22 @@ sdb_measureAccess(const char *directory)
".%lu%s", (PRUint32)(time + i), doesntExistName);
PR_Access(temp, PR_ACCESS_EXISTS);
next = PR_IntervalNow();
delta = next - time;
if (delta >= duration)
break;
}
+#ifdef SDB_MEASURE_USE_TEMP_DIR
+ /* turn temp back into our tmpdir path by removing doesntExistName, and
+ * remove the tmp dir */
+ *tempStartOfFilename = '\0';
+ (void)rmdir(temp);
+#endif
PORT_Free(temp);
/* always return 1 or greater */
return i ? i : 1u;
}
/*
* some file sytems are very slow to run sqlite3 on, particularly if the