Skip to content

Commit 50f14d6

Browse files
authored
[MEMFS] Assume FS.write is takes a TypedArray. (#26413)
We have higher level wrappers `createDataFile` and `writeFile` which also accept `Array` and `string`, but it simpler if the lower level APIs can just assume `TypedArray`. This simplifies the code and reduces code size. We have assertions to check this in debug builds. Followup to #26398 and #26411
1 parent 3625d4b commit 50f14d6

23 files changed

+141
-151
lines changed

ChangeLog.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ See docs/process.md for more on how version tagging works.
2020

2121
5.0.3 (in development)
2222
----------------------
23+
- The low level FS.write API now only accepts TypedArray. The higher level
24+
writeFile and createDataFile file still also accept string and Array.
25+
(#26413)
2326
- Warn on usage of the deprecated `EMSCRIPTEN` macro (`__EMSCRIPTEN__` should
2427
be used instead). (#26381)
2528
- The `-sRELOCATABLE` setting was effectively removed (moved to legacy

src/lib/libcore.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2193,7 +2193,7 @@ addToLibrary({
21932193
#endif
21942194
FS.createPath('/', PATH.dirname(name), true, true);
21952195
// canOwn this data in the filesystem, it is a slice of wasm memory that will never change
2196-
FS.createDataFile(name, null, HEAP8.subarray(content, content + len), true, true, true);
2196+
FS.createDataFile(name, null, HEAP8.subarray(content, content + len), true, true, /*canOwn=*/true);
21972197
} while ({{{ makeGetValue('ptr', '0', '*') }}});
21982198
#if RUNTIME_DEBUG
21992199
dbg('done preloading data files');

src/lib/libfs.js

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
var LibraryFS = {
88
$FS__deps: ['$randomFill', '$PATH', '$PATH_FS', '$TTY', '$MEMFS',
99
'$FS_modeStringToFlags',
10+
'$FS_fileDataToTypedArray',
1011
'$FS_getMode',
1112
'$intArrayFromString',
1213
#if LibraryManager.has('libidbfs.js')
@@ -1253,9 +1254,13 @@ FS.staticInit();`;
12531254
#endif
12541255
return bytesRead;
12551256
},
1257+
/**
1258+
* @param {TypedArray} buffer
1259+
*/
12561260
write(stream, buffer, offset, length, position, canOwn) {
12571261
#if ASSERTIONS
12581262
assert(offset >= 0);
1263+
assert(buffer.subarray, 'FS.write expects a TypedArray');
12591264
#endif
12601265
if (length < 0 || position < 0) {
12611266
throw new FS.ErrnoError({{{ cDefs.EINVAL }}});
@@ -1346,17 +1351,14 @@ FS.staticInit();`;
13461351
FS.close(stream);
13471352
return buf;
13481353
},
1354+
/**
1355+
* @param {TypedArray|Array|string} data
1356+
*/
13491357
writeFile(path, data, opts = {}) {
13501358
opts.flags = opts.flags || {{{ cDefs.O_TRUNC | cDefs.O_CREAT | cDefs.O_WRONLY }}};
13511359
var stream = FS.open(path, opts.flags, opts.mode);
1352-
if (typeof data == 'string') {
1353-
data = new Uint8Array(intArrayFromString(data, true));
1354-
}
1355-
if (ArrayBuffer.isView(data)) {
1356-
FS.write(stream, data, 0, data.byteLength, undefined, opts.canOwn);
1357-
} else {
1358-
abort('Unsupported data type');
1359-
}
1360+
data = FS_fileDataToTypedArray(data);
1361+
FS.write(stream, data, 0, data.byteLength, undefined, opts.canOwn);
13601362
FS.close(stream);
13611363
},
13621364

@@ -1604,6 +1606,9 @@ FS.staticInit();`;
16041606
var mode = FS_getMode(canRead, canWrite);
16051607
return FS.create(path, mode);
16061608
},
1609+
/**
1610+
* @param {TypedArray|Array|string=} data
1611+
*/
16071612
createDataFile(parent, name, data, canRead, canWrite, canOwn) {
16081613
var path = name;
16091614
if (parent) {
@@ -1613,11 +1618,7 @@ FS.staticInit();`;
16131618
var mode = FS_getMode(canRead, canWrite);
16141619
var node = FS.create(path, mode);
16151620
if (data) {
1616-
if (typeof data == 'string') {
1617-
var arr = new Array(data.length);
1618-
for (var i = 0, len = data.length; i < len; ++i) arr[i] = data.charCodeAt(i);
1619-
data = arr;
1620-
}
1621+
data = FS_fileDataToTypedArray(data);
16211622
// make sure we can write to the file
16221623
FS.chmod(node, mode | {{{ cDefs.S_IWUGO }}});
16231624
var stream = FS.open(node, {{{ cDefs.O_TRUNC | cDefs.O_CREAT | cDefs.O_WRONLY }}});

src/lib/libfs_shared.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,16 @@ addToLibrary({
107107
return mode;
108108
},
109109

110+
$FS_fileDataToTypedArray: (data) => {
111+
if (typeof data == 'string') {
112+
data = intArrayFromString(data, true);
113+
}
114+
if (!data.subarray) {
115+
data = new Uint8Array(data);
116+
}
117+
return data;
118+
},
119+
110120
$FS_stdin_getChar_buffer: [],
111121

112122
// getChar has 3 particular return values:

src/lib/libmemfs.js

Lines changed: 25 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -248,16 +248,23 @@ addToLibrary({
248248
return size;
249249
},
250250

251-
// Writes the byte range (buffer[offset], buffer[offset+length]) to offset 'position' into the file pointed by 'stream'
252-
// canOwn: A boolean that tells if this function can take ownership of the passed in buffer from the subbuffer portion
253-
// that the typed array view 'buffer' points to. The underlying ArrayBuffer can be larger than that, but
254-
// canOwn=true will not take ownership of the portion outside the bytes addressed by the view. This means that
255-
// with canOwn=true, creating a copy of the bytes is avoided, but the caller shouldn't touch the passed in range
256-
// of bytes anymore since their contents now represent file data inside the filesystem.
251+
/**
252+
* Writes the byte range (buffer[offset], buffer[offset+length]) to offset
253+
* 'position' into the file pointed by 'stream'.
254+
* @param {TypedArray} buffer
255+
* @param {boolean=} canOwn - A boolean that tells if this function can
256+
* take ownership of the passed in buffer from the subbuffer portion
257+
* that the typed array view 'buffer' points to. The underlying
258+
* ArrayBuffer can be larger than that, but canOwn=true will not take
259+
* ownership of the portion outside the bytes addressed by the view.
260+
* This means that with canOwn=true, creating a copy of the bytes is
261+
* avoided, but the caller shouldn't touch the passed in range of
262+
* bytes anymore since their contents now represent file data inside
263+
* the filesystem.
264+
*/
257265
write(stream, buffer, offset, length, position, canOwn) {
258266
#if ASSERTIONS
259-
// The data buffer should be a typed array view
260-
assert(!(buffer instanceof ArrayBuffer));
267+
assert(buffer.subarray, 'FS.write expects a TypedArray');
261268
#endif
262269
#if ALLOW_MEMORY_GROWTH
263270
// If the buffer is located in main memory (HEAP), and if
@@ -273,35 +280,21 @@ addToLibrary({
273280
var node = stream.node;
274281
node.mtime = node.ctime = Date.now();
275282

276-
if (buffer.subarray) { // This write is from a typed array to a typed array?
277-
if (canOwn) {
283+
if (canOwn) {
278284
#if ASSERTIONS
279-
assert(position === 0, 'canOwn must imply no weird position inside the file');
285+
assert(position === 0, 'canOwn must imply no weird position inside the file');
280286
#endif
281-
node.contents = buffer.subarray(offset, offset + length);
282-
node.usedBytes = length;
283-
return length;
284-
} else if (node.usedBytes === 0 && position === 0) { // If this is a simple first write to an empty file, do a fast set since we don't need to care about old data.
285-
node.contents = buffer.slice(offset, offset + length);
286-
node.usedBytes = length;
287-
return length;
288-
} else if (position + length <= node.usedBytes) { // Writing to an already allocated and used subrange of the file?
289-
node.contents.set(buffer.subarray(offset, offset + length), position);
290-
return length;
291-
}
292-
}
293-
294-
// Appending to an existing file and we need to reallocate, or source data did not come as a typed array.
295-
MEMFS.expandFileStorage(node, position+length);
296-
if (buffer.subarray) {
287+
node.contents = buffer.subarray(offset, offset + length);
288+
node.usedBytes = length;
289+
} else if (node.usedBytes === 0 && position === 0) { // If this is a simple first write to an empty file, do a fast set since we don't need to care about old data.
290+
node.contents = buffer.slice(offset, offset + length);
291+
node.usedBytes = length;
292+
} else {
293+
MEMFS.expandFileStorage(node, position+length);
297294
// Use typed array write which is available.
298295
node.contents.set(buffer.subarray(offset, offset + length), position);
299-
} else {
300-
for (var i = 0; i < length; i++) {
301-
node.contents[position + i] = buffer[offset + i]; // Or fall back to manual write if not.
302-
}
296+
node.usedBytes = Math.max(node.usedBytes, position + length);
303297
}
304-
node.usedBytes = Math.max(node.usedBytes, position + length);
305298
return length;
306299
},
307300

src/lib/libwasmfs.js

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -509,20 +509,17 @@ addToLibrary({
509509
return FS_mknod(path, mode, 0);
510510
},
511511

512-
$FS_writeFile__deps: ['_wasmfs_write_file', '$stackSave', '$stackRestore', 'malloc', 'free'],
512+
$FS_writeFile__deps: ['$FS_fileDataToTypedArray', '_wasmfs_write_file', '$stackSave', '$stackRestore', 'malloc', 'free'],
513513
$FS_writeFile: (path, data) => {
514514
var sp = stackSave();
515515
var pathBuffer = stringToUTF8OnStack(path);
516-
var len = typeof data == 'string' ? lengthBytesUTF8(data) + 1 : data.length;
516+
data = FS_fileDataToTypedArray(data);
517+
var len = data.length;
517518
var dataBuffer = _malloc(len);
518519
#if ASSERTIONS
519520
assert(dataBuffer);
520521
#endif
521-
if (typeof data == 'string') {
522-
len = stringToUTF8(data, dataBuffer, len);
523-
} else {
524-
HEAPU8.set(data, dataBuffer);
525-
}
522+
HEAPU8.set(data, dataBuffer);
526523
var ret = __wasmfs_write_file(pathBuffer, dataBuffer, len);
527524
_free(dataBuffer);
528525
stackRestore(sp);

test/codesize/test_codesize_cxx_ctors1.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
2-
"a.out.js": 19334,
3-
"a.out.js.gz": 8012,
2+
"a.out.js": 19214,
3+
"a.out.js.gz": 7982,
44
"a.out.nodebug.wasm": 132865,
55
"a.out.nodebug.wasm.gz": 49889,
6-
"total": 152199,
7-
"total_gz": 57901,
6+
"total": 152079,
7+
"total_gz": 57871,
88
"sent": [
99
"__cxa_throw",
1010
"_abort_js",

test/codesize/test_codesize_cxx_ctors2.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
2-
"a.out.js": 19311,
3-
"a.out.js.gz": 7998,
2+
"a.out.js": 19191,
3+
"a.out.js.gz": 7968,
44
"a.out.nodebug.wasm": 132285,
55
"a.out.nodebug.wasm.gz": 49542,
6-
"total": 151596,
7-
"total_gz": 57540,
6+
"total": 151476,
7+
"total_gz": 57510,
88
"sent": [
99
"__cxa_throw",
1010
"_abort_js",

test/codesize/test_codesize_cxx_except.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
2-
"a.out.js": 22995,
3-
"a.out.js.gz": 8989,
2+
"a.out.js": 22875,
3+
"a.out.js.gz": 8959,
44
"a.out.nodebug.wasm": 172743,
55
"a.out.nodebug.wasm.gz": 57374,
6-
"total": 195738,
7-
"total_gz": 66363,
6+
"total": 195618,
7+
"total_gz": 66333,
88
"sent": [
99
"__cxa_begin_catch",
1010
"__cxa_end_catch",

test/codesize/test_codesize_cxx_except_wasm.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
2-
"a.out.js": 19145,
3-
"a.out.js.gz": 7934,
2+
"a.out.js": 19025,
3+
"a.out.js.gz": 7906,
44
"a.out.nodebug.wasm": 148138,
55
"a.out.nodebug.wasm.gz": 55261,
6-
"total": 167283,
7-
"total_gz": 63195,
6+
"total": 167163,
7+
"total_gz": 63167,
88
"sent": [
99
"_abort_js",
1010
"_tzset_js",

0 commit comments

Comments
 (0)