1680151 - IOUtils::ReadUTF8 and IOUTils::WriteAtomicUTF8 should use UTF8String instead of DOMString

Status: RESOLVED FIXED

(Toolkit Graveyard :: OS.File, enhancement, P3)

Description

[Beth Rennie [:beth] (she/her)]

This will save us from performing copies when encoding to and from utf8.

Comment 1

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1680151 - Accept UTF8String directly in IOUtils::WriteUTF8 r?gijs,emalysz

Instead of accepting a DOMString (ie, a UTF16 string) and manually converting
it to UTF-8, we can instead accept a UTF8String from JS, which saves us
manually doing conversions and (and may save an additional conversion if the
JSString* is an ASCII string).

Depends on D99002

Comment 2

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1680151 - Reduce copies in IOUtils::Read{,UTF8} r?nika,tcampbell

Previously, in both Read and ReadUTF8, we were doing copies where we did not
need to. Read allocated an nsTArray and passed that to JS, which performed a
copy of its contents to create a Uint8Array. ReadUTF8, on the other hand, would
take that nsTArray, convert it into ns nsString (1 copy), and then pass it to
JS for it to recreate the string (2 copies).

Now, we allocate our string and array buffers up front in JS' memory pools
directly and use the JS API to create the strings and arrays ourselves (instead
of relying on Promise::MaybeResolve() to do a copying conversion). Read now
performs 0 copies in the best case (if the file is not compressed) and ReadUTF8
also does 0 copies in the best case (if the file is not compressed and the
string is ASCII). In the worst case, Read performs a single extra allocation
(to decompress the file) and ReadUTF8 performs 2 (to decompress the file and to
convert a UTF-8 string to either a Latin1 string or a UTF-16 string).

Depends on D99003

Comment 3

[Beth Rennie [:beth] (she/her)]

Attachment: Bug 1680151 - Replace WriteUTF8Sync with call to WriteSync r?nika

Depends on D99004

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f9a606a66973
Accept UTF8String directly in IOUtils::WriteUTF8 r=Gijs
https://hg.mozilla.org/integration/autoland/rev/f5a436352e34
Reduce copies in IOUtils::Read{,UTF8} r=nika,tcampbell
https://hg.mozilla.org/integration/autoland/rev/87222362f8f3
Replace WriteUTF8Sync with call to WriteSync r=nika

Comment 5

[Sandor Molnar[:smolnar]]

Backed out for causing xpc/mochitest failures.

Backout link: https://hg.mozilla.org/integration/autoland/rev/f50c3348905856d9213c507b74aedac4af106b34

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception%2Crunning%2Cpending%2Crunnable&searchStr=linux%2C18.04%2Cx64%2Cdebug%2Cxpcshell%2Ctests%2Ctest-linux1804-64%2Fdebug-xpcshell-e10s%2Cx3&revision=87222362f8f3a9aa75ddfe3922384fa9e561e315

Failure log xpc: https://treeherder.mozilla.org/logviewer?job_id=326689354&repo=autoland&lineNumber=3806

Failure log mochitest: https://treeherder.mozilla.org/logviewer?job_id=326693006&repo=autoland&lineNumber=16641

Failure log mochitest without e10s: https://treeherder.mozilla.org/logviewer?job_id=326692496&repo=autoland&lineNumber=121320

"INFO - TEST-START | toolkit/components/crashes/tests/xpcshell/test_crash_store.js
[task 2021-01-14T14:51:34.591Z] 14:51:34 WARNING - TEST-UNEXPECTED-FAIL | toolkit/components/crashes/tests/xpcshell/test_crash_store.js | xpcshell return code: 0
[task 2021-01-14T14:51:34.592Z] 14:51:34 INFO - TEST-INFO took 235ms
[task 2021-01-14T14:51:34.592Z] 14:51:34 INFO - >>>>>>>
[task 2021-01-14T14:51:34.593Z] 14:51:34 INFO - PID 22147 | Couldn't convert chrome URL: chrome://branding/locale/brand.properties
[task 2021-01-14T14:51:34.593Z] 14:51:34 INFO - PID 22147 | [Parent 22147, Main Thread] WARNING: Couldn't get the user appdata directory. Crash events may not be produced.: file /builds/worker/checkouts/gecko/toolkit/crashreporter/nsExceptionHandler.cpp:2916
[task 2021-01-14T14:51:34.593Z] 14:51:34 INFO - (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2021-01-14T14:51:34.594Z] 14:51:34 INFO - (xpcshell/head.js) | test run_next_test 0 pending (2)
[task 2021-01-14T14:51:34.594Z] 14:51:34 INFO - (xpcshell/head.js) | test MAIN run_test finished (2)
[task 2021-01-14T14:51:34.594Z] 14:51:34 INFO - running event loop
[task 2021-01-14T14:51:34.594Z] 14:51:34 INFO - PID 22147 | [Parent 22147, Main Thread] WARNING: Could not get the program name for a cubeb stream.: 'NS_SUCCEEDED(rv)', file /builds/worker/checkouts/gecko/dom/media/CubebUtils.cpp:387
[task 2021-01-14T14:51:34.595Z] 14:51:34 INFO - toolkit/components/crashes/tests/xpcshell/test_crash_store.js | Starting test_constructor
[task 2021-01-14T14:51:34.595Z] 14:51:34 INFO - (xpcshell/head.js) | test test_constructor pending (2)
[task 2021-01-14T14:51:34.595Z] 14:51:34 INFO - TEST-PASS | toolkit/components/crashes/tests/xpcshell/test_crash_store.js | test_constructor - [test_constructor : 54] true == true"

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dc01552c74c0
Accept UTF8String directly in IOUtils::WriteUTF8 r=Gijs
https://hg.mozilla.org/integration/autoland/rev/166148e5ef4e
Reduce copies in IOUtils::Read{,UTF8} r=nika,tcampbell
https://hg.mozilla.org/integration/autoland/rev/50f790d4be27
Replace WriteUTF8Sync with call to WriteSync r=nika

Comment 7

[Narcis Beleuzu [:NarcisB]]

Backed out for mochitest crashes on IOUtils.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/773314003c52b89f3cb750c12a1e1fc6a9969b87
Log link: https://treeherder.mozilla.org/logviewer?job_id=326752317&repo=autoland&lineNumber=6822

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/337fac3d0b12
Accept UTF8String directly in IOUtils::WriteUTF8 r=Gijs
https://hg.mozilla.org/integration/autoland/rev/b9c8927ff527
Reduce copies in IOUtils::Read{,UTF8} r=nika,tcampbell
https://hg.mozilla.org/integration/autoland/rev/ae066b1ae1d1
Replace WriteUTF8Sync with call to WriteSync r=nika

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/337fac3d0b12
https://hg.mozilla.org/mozilla-central/rev/b9c8927ff527
https://hg.mozilla.org/mozilla-central/rev/ae066b1ae1d1