gh-14990: Pre-allocate session store bytes before writing (gh-14993)

This commit is contained in:
mr. m
2026-08-16 16:15:50 +02:00
committed by GitHub
parent 816ec2d40c
commit 93776ce026
4 changed files with 265 additions and 3 deletions

View File

@@ -0,0 +1,254 @@
diff --git a/browser/components/sessionstore/SessionWriter.sys.mjs b/browser/components/sessionstore/SessionWriter.sys.mjs
--- a/browser/components/sessionstore/SessionWriter.sys.mjs
+++ b/browser/components/sessionstore/SessionWriter.sys.mjs
@@ -80,10 +80,14 @@
return await SessionWriterInternal.wipe();
} finally {
unlock();
}
},
+
+ get _jsonLengthHint() {
+ return SessionWriterInternal._lastJsonLength;
+ },
};
const SessionWriterInternal = {
// Path to the files used by the SessionWriter
Paths: null,
@@ -104,10 +108,14 @@
/**
* Number of old upgrade backups that are being kept
*/
maxUpgradeBackups: null,
+ // Estimated JSON string length from the previous write, used to pre-size
+ // the serialization buffer and avoid incremental reallocations.
+ _lastJsonLength: 0,
+
/**
* Initialize (or reinitialize) the writer.
*
* @param {string} origin Which of sessionstore.js or its backups
* was used. One of the `STATE_*` constants defined above.
@@ -201,48 +209,60 @@
}
}
let startWriteMs = Date.now();
let fileStat;
+ // Add 5% headroom to the hint so small growth between saves doesn't
+ // cause reallocs. The compressed-size-based estimate already has
+ // sufficient margin from the 4x multiplier.
+ let jsonLengthHint = Math.ceil(this._lastJsonLength * 1.05);
+
+ let uncompressedBytes;
if (options.isFinalWrite) {
// We are shutting down. At this stage, we know that
// $Paths.clean is either absent or corrupted. If it was
// originally present and valid, it has been moved to
// $Paths.cleanBackup a long time ago. We can therefore write
// with the guarantees that we erase no important data.
- await IOUtils.writeJSON(this.Paths.clean, state, {
+ uncompressedBytes = await IOUtils.writeJSON(this.Paths.clean, state, {
tmpPath: this.Paths.clean + ".tmp",
compress: true,
+ jsonLengthHint,
});
fileStat = await IOUtils.stat(this.Paths.clean);
} else if (this.state == STATE_RECOVERY) {
// At this stage, either $Paths.recovery was written >= 15
// seconds ago during this session or we have just started
// from $Paths.recovery left from the previous session. Either
// way, $Paths.recovery is good. We can move $Path.backup to
// $Path.recoveryBackup without erasing a good file with a bad
// file.
- await IOUtils.writeJSON(this.Paths.recovery, state, {
+ uncompressedBytes = await IOUtils.writeJSON(this.Paths.recovery, state, {
tmpPath: this.Paths.recovery + ".tmp",
backupFile: this.Paths.recoveryBackup,
compress: true,
+ jsonLengthHint,
});
fileStat = await IOUtils.stat(this.Paths.recovery);
} else {
// In other cases, either $Path.recovery is not necessary, or
// it doesn't exist or it has been corrupted. Regardless,
// don't backup $Path.recovery.
- await IOUtils.writeJSON(this.Paths.recovery, state, {
+ uncompressedBytes = await IOUtils.writeJSON(this.Paths.recovery, state, {
tmpPath: this.Paths.recovery + ".tmp",
compress: true,
+ jsonLengthHint,
});
fileStat = await IOUtils.stat(this.Paths.recovery);
}
telemetry.writeFileMs = Date.now() - startWriteMs;
telemetry.fileSizeBytes = fileStat.size;
+ // Use the actual pre-compression size from this write as the hint
+ // for the next write's buffer allocation.
+ this._lastJsonLength = uncompressedBytes.jsonLength;
lazy.sessionStoreLogger.debug(
`SessionWriter.write wrote ${telemetry.fileSizeBytes} bytes in ${telemetry.writeFileMs}ms`
);
} catch (ex) {
// Don't throw immediately
@@ -375,10 +395,11 @@
} catch (ex) {
exn = exn || ex;
}
this.state = STATE_EMPTY;
+ this._lastJsonLength = 0;
if (exn) {
throw exn;
}
return { result: true };
diff --git a/browser/components/sessionstore/test/unit/test_write_json_length_hint.js b/browser/components/sessionstore/test/unit/test_write_json_length_hint.js
new file mode 100644
--- /dev/null
+++ b/browser/components/sessionstore/test/unit/test_write_json_length_hint.js
@@ -0,0 +1,73 @@
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const { SessionWriter } = ChromeUtils.importESModule(
+ "resource:///modules/sessionstore/SessionWriter.sys.mjs"
+);
+
+const profd = do_get_profile();
+const { SessionFile } = ChromeUtils.importESModule(
+ "resource:///modules/sessionstore/SessionFile.sys.mjs"
+);
+
+const { updateAppInfo } = ChromeUtils.importESModule(
+ "resource://testing-common/AppInfo.sys.mjs"
+);
+updateAppInfo({
+ name: "SessionRestoreTest",
+ ID: "{230de50e-4cd1-11dc-8314-0800200c9a66}",
+ version: "1",
+ platformVersion: "",
+});
+
+add_setup(async function () {
+ let source = do_get_file("data/sessionstore_valid.js");
+ source.copyTo(profd, "sessionstore.js");
+ await writeCompressedFile(
+ SessionFile.Paths.clean.replace("jsonlz4", "js"),
+ SessionFile.Paths.clean
+ );
+ await SessionFile.read();
+});
+
+add_task(async function test_length_hint_updates_after_write() {
+ Assert.equal(
+ SessionWriter._jsonLengthHint,
+ 0,
+ "Length hint starts at 0"
+ );
+
+ await SessionFile.write({});
+
+ let hintAfterSmall = SessionWriter._jsonLengthHint;
+ Assert.equal(
+ hintAfterSmall,
+ JSON.stringify({}).length,
+ "Hint matches the uncompressed JSON byte length"
+ );
+
+ let largerState = await IOUtils.readJSON(
+ PathUtils.join(do_get_cwd().path, "data", "sessionstore_complete.json")
+ );
+ await SessionFile.write(largerState);
+
+ Assert.greater(
+ SessionWriter._jsonLengthHint,
+ hintAfterSmall,
+ "Hint grows after writing a larger state"
+ );
+});
+
+add_task(async function test_length_hint_resets_on_wipe() {
+ await SessionFile.write({ windows: [{ tabs: [{ entries: [] }] }] });
+ Assert.greater(SessionWriter._jsonLengthHint, 0, "Hint is nonzero");
+
+ await SessionFile.wipe();
+ Assert.equal(
+ SessionWriter._jsonLengthHint,
+ 0,
+ "Hint resets to 0 after wipe"
+ );
+});
diff --git a/browser/components/sessionstore/test/unit/xpcshell.toml b/browser/components/sessionstore/test/unit/xpcshell.toml
--- a/browser/components/sessionstore/test/unit/xpcshell.toml
+++ b/browser/components/sessionstore/test/unit/xpcshell.toml
@@ -39,5 +39,10 @@
skip-if = [
"condprof", # Bug 1769154
]
["test_startup_session_async.js"]
+
+["test_write_json_length_hint.js"]
+support-files = [
+ "data/sessionstore_complete.json",
+]
diff --git a/dom/chrome-webidl/IOUtils.webidl b/dom/chrome-webidl/IOUtils.webidl
--- a/dom/chrome-webidl/IOUtils.webidl
+++ b/dom/chrome-webidl/IOUtils.webidl
@@ -101,12 +101,12 @@
*
* @param path An absolute file path
* @param value The value to be serialized.
* @param options Options for writing the file. The "append" mode is not supported.
*
- * @return Resolves with the number of bytes successfully written to the file,
- * otherwise rejects with a DOMException.
+ * @return Resolves with the pre-compression size of the serialized JSON in
+ * bytes (UTF-8), otherwise rejects with a DOMException.
*/
[NewObject]
Promise<WriteJSONResult> writeJSON(DOMString path, any value, optional WriteJSONOptions options = {});
/**
* Moves the file from |sourcePath| to |destPath|, creating necessary parents.
@@ -564,10 +564,16 @@
boolean flush = false;
/**
* If true, compress the data with LZ4-encoding before writing to the file.
*/
boolean compress = false;
+ /**
+ * For |writeJSON|, a hint for the expected JSON string length in UTF-16 code
+ * units. When provided, the JSON serializer pre-allocates a buffer of this
+ * size to avoid incremental reallocations.
+ */
+ unsigned long long jsonLengthHint = 0;
};
/**
* Options to be passed to the |IOUtils.writeJSON| method.
*/
diff --git a/xpcom/ioutils/IOUtils.cpp b/xpcom/ioutils/IOUtils.cpp
--- a/xpcom/ioutils/IOUtils.cpp
+++ b/xpcom/ioutils/IOUtils.cpp
@@ -629,10 +629,13 @@
}
JSContext* cx = aGlobal.Context();
JS::Rooted<JS::Value> value(cx, aValue);
nsString string;
+ if (opts.mLengthHint) {
+ string.SetCapacity(opts.mLengthHint);
+ }
if (!JS_StringifyWithLengthHint(cx, &value, nullptr,
JS::NullHandleValue, AppendJSON,
&string, opts.mLengthHint)) {
JS::Rooted<JS::Value> exn(cx, JS::UndefinedValue());
if (JS_GetPendingException(cx, &exn)) {

View File

@@ -39,5 +39,13 @@
"type": "phabricator",
"id": "D312091",
"name": "Expose tiled attribute to all platforms"
},
{
"type": "phabricator",
"id": "D298708",
"name": "Issue 14990",
"replaces": {
"this._lastJsonLength = uncompressedBytes;": "this._lastJsonLength = uncompressedBytes.jsonLength;"
}
}
]

View File

@@ -1,5 +1,5 @@
diff --git a/toolkit/modules/JSONFile.sys.mjs b/toolkit/modules/JSONFile.sys.mjs
index 397991e4af8f49b6365d729fc11267b5c1113400..9b1d6fd3850b239000a3c4d2a2d5799a0989f4e3 100644
index 397991e4af8f49b6365d729fc11267b5c1113400..70da83a0dd7658986dfd5f52103873bbb33b3c18 100644
--- a/toolkit/modules/JSONFile.sys.mjs
+++ b/toolkit/modules/JSONFile.sys.mjs
@@ -132,6 +132,7 @@ export function JSONFile(config) {
@@ -23,7 +23,7 @@ index 397991e4af8f49b6365d729fc11267b5c1113400..9b1d6fd3850b239000a3c4d2a2d5799a
this._data,
Object.assign({ tmpPath: this.path + ".tmp" }, this._options)
);
+ this._lastSavedSize = this._useSizeHints ? result : null;
+ this._lastSavedSize = this._useSizeHints ? result.jsonLength : null;
} catch (ex) {
if (typeof this._data.toJSONSafe == "function") {
// If serialization fails, try fallback safe JSON converter.

View File

@@ -46,8 +46,8 @@
min-height: 30px;
}
/* Firefox View */
#firefox-view-button,
#ai-window-toggle,
#wrapper-firefox-view-button {
display: none !important;
}