Support importing profiles whose contents exceed V8s maximum string size #385

jlfwong · 2022-03-27T09:10:32Z

Browsers have a limit on how big you can make strings. In Chrome on a 64 bit machine, this is around 512MB, which explains why in #340, a 600MB file fails to load.

To work around this issue, we avoid making strings this large.

To do so, we need two core changes:

Instead of sending large strings as the import mechanism to different file format importers, we introduce a new TextFileContent interface which exposes methods to get the lines in the file or the JSON representation. In the case of line splitting, we assume that no single line exceeds the 512MB limit.
We introduce a dependency on https://github.com/evanw/uint8array-json-parser to allow us to parse JSON files contained in Uint8Array objects

To ensure that this code doesn't code rot without introducing 600MB test files or test file generation into the repository, we also re-run a small set of tests with a mocked maximum string size of 100 bytes. You can see that the chunked string representation code is getting executed via test coverage.

Fixes #340

coveralls · 2022-03-27T09:12:35Z

Coverage increased (+0.6%) to 41.664% when pulling 5f851a4 on jlfwong/big-string into e37f6fa on main.

bjorn3 · 2022-03-27T16:23:30Z

package.json

@@ -53,6 +53,7 @@
    "ts-jest": "24.3.0",
    "typescript": "4.2.3",
    "typescript-json-schema": "0.42.0",
+    "uint8array-json-parser": "^0.0.1",


Why would adding a single dependency add 15000 lines to the package-lock.json file?

Huh. It's a good question.

My guess is something to do with the lockfileVersion change: https://docs.npmjs.com/cli/v8/configuring-npm/package-lock-json#lockfileversion

Before this change, the package-lock.json was lockfileVersion: 1, and after the chang eit was lockfileVersion: 2. This has nothing to do with this change, it just so happens this is the first time I've installed something since the npm version change.

…#408) In #385, I introduced a dependency on the `uint8array-json-parser` npm package, but used a fork because of a typescript error. This was resolved in evanw/uint8array-json-parser#1 and published as part of `uint8array-json-parser@0.0.2`. Let's use the upstream. This also conveniently fixes a new typechecking error that was preventing deployment. The error looked like this: ``` src/import/utils.ts(2,26): error TS2306: File '\''/Users/jlfwong/code/speedscope/node_modules/uint8array-json-parser/uint8array-json-parser.ts'\'' is not a module.' ``` After updating to the upstream, the problem is fixed.

Currently, importing files generated by linux perf tool whose some blocks exceed V8 strings limit can crash the application. This issue is similar to the one in jlfwong#385. This PR fixes it by changing parseEvents to work directly with lines instead of chunking lines into blocks first. Fixes jlfwong#433

Currently, importing files generated by linux perf tool whose some blocks exceed V8 strings limit can crash the application. This issue is similar to the one in #385. This PR fixes it by changing parseEvents to work directly with lines instead of chunking lines into blocks first. Fixes #433

…ize (jlfwong#385) Browsers have a limit on how big you can make strings. In Chrome on a 64 bit machine, this is around 512MB, which explains why in jlfwong#340, a 600MB file fails to load. To work around this issue, we avoid making strings this large. To do so, we need two core changes: 1. Instead of sending large strings as the import mechanism to different file format importers, we introduce a new `TextFileContent` interface which exposes methods to get the lines in the file or the JSON representation. In the case of line splitting, we assume that no single line exceeds the 512MB limit. 2. We introduce a dependency on https://github.com/evanw/uint8array-json-parser to allow us to parse JSON files contained in `Uint8Array` objects To ensure that this code doesn't code rot without introducing 600MB test files or test file generation into the repository, we also re-run a small set of tests with a mocked maximum string size of 100 bytes. You can see that the chunked string representation code is getting executed via test coverage. Fixes jlfwong#340

…jlfwong#408) In jlfwong#385, I introduced a dependency on the `uint8array-json-parser` npm package, but used a fork because of a typescript error. This was resolved in evanw/uint8array-json-parser#1 and published as part of `uint8array-json-parser@0.0.2`. Let's use the upstream. This also conveniently fixes a new typechecking error that was preventing deployment. The error looked like this: ``` src/import/utils.ts(2,26): error TS2306: File '\''/Users/jlfwong/code/speedscope/node_modules/uint8array-json-parser/uint8array-json-parser.ts'\'' is not a module.' ``` After updating to the upstream, the problem is fixed.

Currently, importing files generated by linux perf tool whose some blocks exceed V8 strings limit can crash the application. This issue is similar to the one in jlfwong#385. This PR fixes it by changing parseEvents to work directly with lines instead of chunking lines into blocks first. Fixes jlfwong#433

jlfwong added 8 commits March 27, 2022 00:55

Add uint8array-json-parser as dependency

ba71928

Support JSON files larger than string limit

18d7d66

600MB linux perf profile loads successfully

2aa8e3b

Add firstChunk interface

ca857bd

Update BG flamegraph

cc051c5

Add support for large instruments deep copy files

025ffcb

Remove asString() method from TextFileContent

ffb3dcc

Remove duplicate parsing

4d88205

jlfwong added 6 commits March 27, 2022 02:13

Run prettier

8b5ed29

Fix linux perf import, start adding tests

9da6291

Make fixUpJSON strategy work for the Uint8Array JSON parsing as well

e7664e7

Factor out JSON fixup code, use it in one more place

9270c29

Resolve TODO

9450b13

Add TODO to CLI

62c6452

bjorn3 reviewed Mar 27, 2022

View reviewed changes

jlfwong added 2 commits May 16, 2022 22:06

Add tests for TextFileContent

51a6fdc

Use fork of uint8array-json-parser to resolve type & dependency issues

ca5f9cf

jlfwong mentioned this pull request May 17, 2022

Make module consumable via TypeScript in other projects evanw/uint8array-json-parser#1

Merged

Warn when CLI tries to load a file that's too large

5f851a4

jlfwong marked this pull request as ready for review May 17, 2022 06:10

jlfwong changed the title ~~WIP: Support importing profiles whose contents exceed V8s maximum string size~~ Support importing profiles whose contents exceed V8s maximum string size May 17, 2022

jlfwong merged commit 21167e6 into main May 17, 2022

jlfwong deleted the jlfwong/big-string branch May 17, 2022 06:11

jlfwong mentioned this pull request Oct 21, 2022

Update package.json to use upstream version of uint8array-json-parser #408

Merged

Goose97 mentioned this pull request Jun 27, 2023

Importing a large linux perf file will crash the application #433

Closed

Goose97 mentioned this pull request Jun 28, 2023

Fix crash when importing big linux perf tool files #435

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support importing profiles whose contents exceed V8s maximum string size #385

Support importing profiles whose contents exceed V8s maximum string size #385

jlfwong commented Mar 27, 2022 •

edited

Loading

coveralls commented Mar 27, 2022 •

edited

Loading

bjorn3 Mar 27, 2022

jlfwong Mar 27, 2022

Support importing profiles whose contents exceed V8s maximum string size #385

Support importing profiles whose contents exceed V8s maximum string size #385

Conversation

jlfwong commented Mar 27, 2022 • edited Loading

coveralls commented Mar 27, 2022 • edited Loading

bjorn3 Mar 27, 2022

Choose a reason for hiding this comment

jlfwong Mar 27, 2022

Choose a reason for hiding this comment

jlfwong commented Mar 27, 2022 •

edited

Loading

coveralls commented Mar 27, 2022 •

edited

Loading