RESOLVED FIXED 174727
Bmalloc and GC should put auxiliaries (butterflies, typed array backing stores) in a gigacage (separate multi-GB VM region)
https://bugs.webkit.org/show_bug.cgi?id=174727
Summary Bmalloc and GC should put auxiliaries (butterflies, typed array backing store...
Filip Pizlo
Reported 2017-07-21 14:39:49 PDT
So fun.
Attachments
it's coming along! (37.10 KB, patch)
2017-07-21 14:40 PDT, Filip Pizlo
no flags
a little more (52.56 KB, patch)
2017-07-21 18:40 PDT, Filip Pizlo
no flags
couldn't help myself (76.67 KB, patch)
2017-07-21 19:20 PDT, Filip Pizlo
no flags
more (101.40 KB, patch)
2017-07-21 21:32 PDT, Filip Pizlo
no flags
pretty good initial version (87.65 KB, patch)
2017-07-22 10:12 PDT, Filip Pizlo
no flags
starting to compile (91.30 KB, patch)
2017-07-22 10:42 PDT, Filip Pizlo
no flags
JSC compiles! (107.02 KB, patch)
2017-07-22 16:29 PDT, Filip Pizlo
no flags
it can print hello! (107.41 KB, patch)
2017-07-22 16:54 PDT, Filip Pizlo
no flags
it does sensible things for a program that accesses arrays! (109.48 KB, patch)
2017-07-22 18:17 PDT, Filip Pizlo
no flags
more (122.70 KB, patch)
2017-07-23 17:55 PDT, Filip Pizlo
no flags
the patch (127.12 KB, patch)
2017-07-23 19:22 PDT, Filip Pizlo
buildbot: commit-queue-
Archive of layout-test-results from ews105 for mac-elcapitan-wk2 (1.06 MB, application/zip)
2017-07-23 20:33 PDT, Build Bot
no flags
Archive of layout-test-results from ews102 for mac-elcapitan (1.23 MB, application/zip)
2017-07-23 20:38 PDT, Build Bot
no flags
Archive of layout-test-results from ews115 for mac-elcapitan (1.21 MB, application/zip)
2017-07-23 20:46 PDT, Build Bot
no flags
Archive of layout-test-results from ews121 for ios-simulator-wk2 (988.82 KB, application/zip)
2017-07-23 20:56 PDT, Build Bot
no flags
passing more tests (134.91 KB, patch)
2017-07-24 20:59 PDT, Filip Pizlo
no flags
rebased (138.25 KB, patch)
2017-07-25 19:01 PDT, Filip Pizlo
no flags
getting closer (146.39 KB, patch)
2017-07-25 20:24 PDT, Filip Pizlo
no flags
maybe it's done now (165.57 KB, patch)
2017-07-25 20:57 PDT, Filip Pizlo
no flags
it compiles! (174.80 KB, patch)
2017-07-25 21:30 PDT, Filip Pizlo
no flags
it runs wasmbench! (199.66 KB, patch)
2017-07-27 11:47 PDT, Filip Pizlo
no flags
came up with a more principled way of handling null (201.66 KB, patch)
2017-07-27 14:19 PDT, Filip Pizlo
no flags
fixed more wasm memory bugs (201.78 KB, patch)
2017-07-27 15:12 PDT, Filip Pizlo
no flags
it has a chance of passing JSC tests (209.93 KB, patch)
2017-07-27 17:12 PDT, Filip Pizlo
no flags
passes JSC tests (209.94 KB, patch)
2017-07-27 18:14 PDT, Filip Pizlo
no flags
rebased (209.94 KB, patch)
2017-07-27 18:32 PDT, Filip Pizlo
no flags
fix 32-bit (209.96 KB, patch)
2017-07-27 19:18 PDT, Filip Pizlo
no flags
try to fix gcc build (209.95 KB, patch)
2017-07-27 19:27 PDT, Filip Pizlo
no flags
fix more builds (210.08 KB, patch)
2017-07-27 19:34 PDT, Filip Pizlo
no flags
the patch (211.61 KB, patch)
2017-07-27 20:01 PDT, Filip Pizlo
no flags
more fixes (212.18 KB, patch)
2017-07-27 20:26 PDT, Filip Pizlo
no flags
more (212.39 KB, patch)
2017-07-27 20:30 PDT, Filip Pizlo
no flags
more (212.39 KB, patch)
2017-07-27 21:16 PDT, Filip Pizlo
buildbot: commit-queue-
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 (1.13 MB, application/zip)
2017-07-27 22:37 PDT, Build Bot
no flags
fixed metal (215.69 KB, patch)
2017-07-28 15:40 PDT, Filip Pizlo
buildbot: commit-queue-
Archive of layout-test-results from ews100 for mac-elcapitan (1.01 MB, application/zip)
2017-07-28 18:19 PDT, Build Bot
no flags
fix some issues (215.94 KB, patch)
2017-07-29 12:14 PDT, Filip Pizlo
no flags
the patch (216.04 KB, patch)
2017-07-29 12:52 PDT, Filip Pizlo
no flags
trial patch (216.10 KB, patch)
2017-07-29 13:29 PDT, Filip Pizlo
no flags
the patch (216.05 KB, patch)
2017-07-29 13:57 PDT, Filip Pizlo
no flags
the patch (216.07 KB, patch)
2017-07-29 14:47 PDT, Filip Pizlo
no flags
the patch (216.09 KB, patch)
2017-07-29 15:41 PDT, Filip Pizlo
no flags
maybe this will do it (216.13 KB, patch)
2017-07-29 17:23 PDT, Filip Pizlo
no flags
rebased (223.28 KB, patch)
2017-08-01 12:16 PDT, Filip Pizlo
no flags
with more fixes (223.20 KB, patch)
2017-08-01 13:13 PDT, Filip Pizlo
mark.lam: review+
buildbot: commit-queue-
Archive of layout-test-results from ews124 for ios-simulator-wk2 (975.30 KB, application/zip)
2017-08-01 14:59 PDT, Build Bot
no flags
Archive of layout-test-results from ews117 for mac-elcapitan (1.87 MB, application/zip)
2017-08-01 15:03 PDT, Build Bot
no flags
Filip Pizlo
Comment 1 2017-07-21 14:40:39 PDT
Created attachment 316124 [details] it's coming along!
Filip Pizlo
Comment 2 2017-07-21 18:40:29 PDT
Created attachment 316156 [details] a little more
Filip Pizlo
Comment 3 2017-07-21 19:20:18 PDT
Created attachment 316161 [details] couldn't help myself
Filip Pizlo
Comment 4 2017-07-21 21:32:34 PDT
Filip Pizlo
Comment 5 2017-07-22 10:12:47 PDT
Created attachment 316199 [details] pretty good initial version I haven't tried compiling it yet.
Filip Pizlo
Comment 6 2017-07-22 10:42:10 PDT
Created attachment 316200 [details] starting to compile
Filip Pizlo
Comment 7 2017-07-22 16:29:37 PDT
Created attachment 316210 [details] JSC compiles!
Filip Pizlo
Comment 8 2017-07-22 16:54:48 PDT
Created attachment 316212 [details] it can print hello!
Filip Pizlo
Comment 9 2017-07-22 18:17:48 PDT
Created attachment 316221 [details] it does sensible things for a program that accesses arrays!
Filip Pizlo
Comment 10 2017-07-23 17:55:29 PDT
Filip Pizlo
Comment 11 2017-07-23 19:22:37 PDT
Created attachment 316250 [details] the patch I think it's passing tests now.
Build Bot
Comment 12 2017-07-23 19:24:32 PDT
Attachment 316250 [details] did not pass style-queue: ERROR: Source/WTF/wtf/Gigacage.h:60: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:117: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:188: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Gigacage.cpp:30: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Heap.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/VM.cpp:423: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/heap/Subspace.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 11 in 58 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 13 2017-07-23 19:27:26 PDT
Comment on attachment 316250 [details] the patch It's not ready for review yet. I'm seeing some test failures.
Filip Pizlo
Comment 14 2017-07-23 20:01:34 PDT
Ooops, I forgot about wasm memory.
Filip Pizlo
Comment 15 2017-07-23 20:05:51 PDT
I think I can get this to work by just rewriting how WasmMemory works. It should just use the Gigacage. We should probably just have the Gigacage be structured as follows: - 64GB for the normal Gigacage - 64GB for WebAssembly memories Then we just need a GC control system that can juggle 16 wasm memories gracefully. Sounds pretty easy.
Build Bot
Comment 16 2017-07-23 20:33:56 PDT
Comment on attachment 316250 [details] the patch Attachment 316250 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4176019 Number of test failures exceeded the failure limit.
Build Bot
Comment 17 2017-07-23 20:33:59 PDT
Created attachment 316255 [details] Archive of layout-test-results from ews105 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Build Bot
Comment 18 2017-07-23 20:38:23 PDT
Comment on attachment 316250 [details] the patch Attachment 316250 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/4176016 Number of test failures exceeded the failure limit.
Build Bot
Comment 19 2017-07-23 20:38:24 PDT
Created attachment 316257 [details] Archive of layout-test-results from ews102 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Build Bot
Comment 20 2017-07-23 20:41:49 PDT
Comment on attachment 316250 [details] the patch Attachment 316250 [details] did not pass jsc-ews (mac): Output: http://webkit-queues.webkit.org/results/4176013 New failing tests: stress/typed-array-byte-offset.js.default wasm.yaml/wasm/function-tests/loop-mult.js.default-wasm stress/put-direct-index-broken-2.js.no-llint wasm.yaml/wasm/self-test/test_LowLevelBinary_varuint1.js.wasm-eager-jettison jsc-layout-tests.yaml/js/script-tests/regress-155776.js.layout-no-llint wasm.yaml/wasm/function-tests/i32-const.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/load-offset.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/Module.customSection.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/invalid-duplicate-export.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/memory-access-past-4gib.js.wasm-eager-jettison microbenchmarks/asmjs_bool_bug.js.no-cjit-collect-continuously wasm.yaml/wasm/js-api/global-mutate.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/trap-load-2.js.wasm-no-tls-context wasm.yaml/wasm/js-api/test_Instance.js.default-wasm stress/jsc-read.js.ftl-no-cjit-validate-sampling-profiler wasm.yaml/wasm/function-tests/rotl.js.wasm-no-tls-context stress/jsc-read.js.no-cjit-validate-phases wasm.yaml/wasm/js-api/validate.js.wasm-no-tls-context wasm.yaml/wasm/js-api/Module.customSection.js.default-wasm wasm.yaml/wasm/js-api/test_Data.js.wasm-eager-jettison stress/typed-array-byte-offset.js.dfg-eager wasm.yaml/wasm/js-api/Instance.imports.exports.unicode.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/add-12.js.default-wasm wasm.yaml/wasm/function-tests/if-no-else-non-void.js.wasm-no-tls-context wasm.yaml/wasm/js-api/wasm-to-wasm.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/many-arguments-to-function.js.wasm-no-tls-context wasm.yaml/wasm/js-api/globals-import.js.wasm-no-tls-context wasm.yaml/wasm/js-api/globals-export.js.default-wasm wasm.yaml/wasm/function-tests/f32-const.js.default-wasm wasm.yaml/wasm/fuzz/export-function.js.default-wasm microbenchmarks/typed-array-subarray.js.ftl-eager-no-cjit-b3o1 wasm.yaml/wasm/js-api/Module.exports.js.wasm-no-tls-context jsc-layout-tests.yaml/js/script-tests/regress-155776.js.layout stress/jsc-read.js.no-llint wasm.yaml/wasm/js-api/web-assembly-instantiate-parallel.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/grow-memory.js.default-wasm wasm.yaml/wasm/js-api/wasm-to-wasm-bad-signature.js.wasm-no-tls-context stress/typed-array-byte-offset.js.ftl-no-cjit-no-inline-validate wasm.yaml/wasm/js-api/global-error.js.wasm-no-cjit-yes-tls-context jsc-layout-tests.yaml/js/script-tests/regress-155776.js.layout-ftl-no-cjit wasm.yaml/wasm/function-tests/factorial.js.default-wasm wasm.yaml/wasm/function-tests/float-sub.js.wasm-no-cjit-yes-tls-context stress/jsc-read.js.dfg-eager-no-cjit-validate wasm.yaml/wasm/js-api/element.js.wasm-no-tls-context wasm.yaml/wasm/js-api/test_Start.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/table-basic.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/i32-const.js.wasm-no-tls-context microbenchmarks/typed-array-subarray.js.no-ftl wasm.yaml/wasm/function-tests/many-args-tail-call-sp-restored.js.wasm-no-call-ic wasm.yaml/wasm/js-api/globals-import.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/grow-memory-3.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/memory-import-and-grow.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/struct.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/f64-const.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/context-switch.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/nameSection.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/many-args-tail-call-sp-restored.js.wasm-eager-jettison wasm.yaml/wasm/js-api/export-arity.js.default-wasm wasm.yaml/wasm/function-tests/load-offset.js.wasm-eager-jettison wasm.yaml/wasm/fuzz/memory.js.wasm-eager-jettison stress/jsc-read.js.ftl-no-cjit-no-inline-validate wasm.yaml/wasm/js-api/test_Data.js.default-wasm wasm.yaml/wasm/spec-tests/jsapi.js.wasm-no-tls-context microbenchmarks/typed-array-get-set-by-val-profiling.js.ftl-eager-no-cjit-b3o1 wasm.yaml/wasm/function-tests/i32-load8-s.js.wasm-no-tls-context wasm.yaml/wasm/js-api/memory-grow.js.default-wasm wasm.yaml/wasm/function-tests/trap-after-cross-instance-call.js.default-wasm microbenchmarks/asmjs_bool_bug.js.dfg-eager-no-cjit-validate wasm.yaml/wasm/function-tests/load-offset.js.default-wasm wasm.yaml/wasm/js-api/global-external-init-from-import.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/test_Data.js.wasm-no-cjit-yes-tls-context microbenchmarks/typed-array-get-set-by-val-profiling.js.ftl-eager-no-cjit wasm.yaml/wasm/js-api/table.js.wasm-no-tls-context wasm.yaml/wasm/self-test/test_LowLevelBinary_uint16.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/basic-element.js.wasm-no-tls-context wasm.yaml/wasm/fuzz/export-function.js.wasm-no-tls-context wasm.yaml/wasm/js-api/web-assembly-instantiate-parallel.js.default-wasm wasm.yaml/wasm/function-tests/function-import-return-value.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/brTableManyValues.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/grow-memory-4.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/brTableWithLoop.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/float-sub.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/factorial.js.wasm-eager-jettison wasm.yaml/wasm/js-api/wrapper-function.js.default-wasm wasm.yaml/wasm/spec-tests/jsapi.js.default-wasm wasm.yaml/wasm/function-tests/stack-trace.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/unique-signature.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/br-table-as-return.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/grow-memory-4.js.wasm-no-tls-context wasm.yaml/wasm/self-test/test_BuilderWebAssembly.js.wasm-eager-jettison wasm.yaml/wasm/self-test/test_BuilderWebAssembly.js.default-wasm wasm.yaml/wasm/function-tests/brTableManyValues.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/if-no-else-non-void.js.wasm-no-cjit-yes-tls-context stress/typed-array-byte-offset.js.no-ftl jsc-layout-tests.yaml/js/script-tests/regress-155776.js.layout-dfg-eager-no-cjit wasm.yaml/wasm/function-tests/table-basic.js.wasm-eager-jettison stress/put-direct-index-broken-2.js.dfg-eager wasm.yaml/wasm/function-tests/load-offset.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/nameSection.js.default-wasm microbenchmarks/typed-array-get-set-by-val-profiling.js.ftl-no-cjit-no-inline-validate stress/jsc-read.js.dfg-maximal-flush-validate-no-cjit wasm.yaml/wasm/function-tests/memory-grow-invalid.js.wasm-no-tls-context microbenchmarks/typed-array-get-set-by-val-profiling.js.dfg-maximal-flush-validate-no-cjit wasm.yaml/wasm/js-api/Module.exports.js.wasm-no-call-ic wasm.yaml/wasm/js-api/export-void-is-undef.js.default-wasm wasm.yaml/wasm/function-tests/trap-after-cross-instance-call.js.wasm-no-tls-context wasm.yaml/wasm/js-api/validate.js.default-wasm wasm.yaml/wasm/function-tests/shr-u.js.default-wasm wasm.yaml/wasm/js-api/global-mutate.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/brTableManyValues.js.default-wasm wasm.yaml/wasm/function-tests/f64-const.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/grow-memory.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/shl.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/factorial.js.wasm-no-tls-context microbenchmarks/typed-array-subarray.js.no-cjit-validate-phases wasm.yaml/wasm/js-api/call-indirect.js.wasm-no-cjit-yes-tls-context stress/put-direct-index-broken-2.js.ftl-eager stress/typedarray-slice.js.dfg-maximal-flush-validate-no-cjit wasm.yaml/wasm/function-tests/table-basic-2.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/brTableAsIf.js.wasm-no-call-ic wasm.yaml/wasm/lowExecutableMemory/exports-oom.js.default-wasm wasm.yaml/wasm/function-tests/invalid-duplicate-export.js.default-wasm wasm.yaml/wasm/js-api/memory-grow.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/brTableWithLoop.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/many-arguments-to-function.js.wasm-no-call-ic wasm.yaml/wasm/js-api/Module.imports.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/trap-load.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/rotl.js.wasm-no-call-ic stress/typed-array-byte-offset.js.dfg-maximal-flush-validate-no-cjit microbenchmarks/typed-array-get-set-by-val-profiling.js.no-ftl wasm.yaml/wasm/function-tests/nameSection.js.wasm-no-call-ic wasm.yaml/wasm/js-api/test_Module.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/self-test/test_LowLevelBinary_uint24.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/struct.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/table-basic.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/factorial.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/i32-const.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/trap-store-2.js.wasm-no-call-ic wasm.yaml/wasm/js-api/test_memory.js.wasm-no-tls-context wasm.yaml/wasm/js-api/Module.exports.js.default-wasm wasm.yaml/wasm/function-tests/memory-access-past-4gib.js.default-wasm wasm.yaml/wasm/js-api/call-indirect.js.default-wasm wasm.yaml/wasm/js-api/element-data.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/context-switch.js.wasm-no-cjit-yes-tls-context stress/typedarray-slice.js.ftl-no-cjit-b3o1 wasm.yaml/wasm/function-tests/memory-grow-invalid.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/i32-load.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/loop-mult.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/web-assembly-compile-parallel.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/rotl.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/br-if-as-return.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/shr-s.js.wasm-no-tls-context wasm.yaml/wasm/js-api/element.js.wasm-no-cjit-yes-tls-context stress/typedarray-slice.js.ftl-eager wasm.yaml/wasm/function-tests/f32-const.js.wasm-no-tls-context wasm.yaml/wasm/js-api/table.js.wasm-eager-jettison wasm.yaml/wasm/self-test/test_LowLevelBinary_varuint32.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/test_Module.js.wasm-eager-jettison wasm.yaml/wasm/self-test/test_LowLevelBinary_uint32.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/br-if-loop-less-than.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/grow-memory.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/add-12.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/test_Start.js.default-wasm wasm.yaml/wasm/function-tests/i32-load8-s.js.wasm-no-call-ic stress/put-direct-index-broken-2.js.ftl-eager-no-cjit-b3o1 wasm.yaml/wasm/js-api/web-assembly-function.js.wasm-no-tls-context wasm.yaml/wasm/js-api/global-mutate.js.wasm-no-tls-context stress/put-direct-index-broken-2.js.ftl-no-cjit-b3o1 wasm.yaml/wasm/fuzz/memory.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/struct.js.wasm-eager-jettison wasm.yaml/wasm/js-api/test_Instance.js.wasm-eager-jettison wasm.yaml/wasm/js-api/extension-MemoryMode.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/if-no-else-non-void.js.wasm-eager-jettison wasm.yaml/wasm/self-test/test_LowLevelBinary_uint32.js.wasm-eager-jettison stress/typedarray-slice.js.ftl-no-cjit-no-put-stack-validate wasm.yaml/wasm/fuzz/memory.js.default-wasm wasm.yaml/wasm/function-tests/trap-store.js.wasm-no-tls-context microbenchmarks/asmjs_bool_bug.js.ftl-eager-no-cjit-b3o1 microbenchmarks/asmjs_bool_bug.js.ftl-no-cjit-small-pool wasm.yaml/wasm/js-api/call-indirect.js.wasm-no-tls-context wasm.yaml/wasm/js-api/web-assembly-instantiate-parallel.js.wasm-no-call-ic wasm.yaml/wasm/js-api/test_memory_constructor.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/rotr.js.default-wasm wasm.yaml/wasm/js-api/globals-import.js.default-wasm stress/jsc-read.js.ftl-eager wasm.yaml/wasm/function-tests/exceptions.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/rotr.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/ret5.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/memory-alignment.js.wasm-eager-jettison wasm.yaml/wasm/js-api/web-assembly-instantiate.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/export-void-is-undef.js.wasm-no-call-ic wasm.yaml/wasm/lowExecutableMemory/executable-memory-oom.js.default-wasm microbenchmarks/typed-array-get-set-by-val-profiling.js.ftl-no-cjit-small-pool wasm.yaml/wasm/self-test/test_LowLevelBinary_varint32.js.wasm-eager-jettison stress/typedarray-slice.js.ftl-no-cjit-no-inline-validate wasm.yaml/wasm/js-api/Module.customSection.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/float-sub.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.wasm-no-tls-context stress/jsc-read.js.ftl-eager-no-cjit microbenchmarks/typed-array-subarray.js.default wasm.yaml/wasm/function-tests/br-as-return.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/jsapi.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/grow-memory-2.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/nameSection.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/trap-load-2.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/br-table-as-return.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/wrapper-function.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/globals-export.js.wasm-no-tls-context wasm.yaml/wasm/js-api/extension-MemoryMode.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/grow-memory.js.wasm-eager-jettison stress/put-direct-index-broken-2.js.dfg-eager-no-cjit-validate jsc-layout-tests.yaml/js/script-tests/regress-155776.js.layout-no-cjit wasm.yaml/wasm/function-tests/basic-element.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/shr-s.js.wasm-no-cjit-yes-tls-context stress/typedarray-slice.js.ftl-eager-no-cjit-b3o1 wasm.yaml/wasm/js-api/wasm-to-wasm-bad-signature.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/invalid-duplicate-export.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/f64-const.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/loop-sum.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/basic-element.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/brTableAsIf.js.wasm-no-cjit-yes-tls-context stress/typed-array-byte-offset.js.ftl-no-cjit-b3o1 stress/typed-array-byte-offset.js.ftl-no-cjit-no-put-stack-validate wasm.yaml/wasm/function-tests/function-import-return-value.js.default-wasm wasm.yaml/wasm/js-api/dont-mmap-zero-byte-memory.js.wasm-no-call-ic stress/typed-array-byte-offset.js.no-llint wasm.yaml/wasm/function-tests/function-import-return-value.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/memory-grow-invalid.js.wasm-eager-jettison wasm.yaml/wasm/js-api/table.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/ret5.js.wasm-no-tls-context wasm.yaml/wasm/js-api/global-internal-init-from-import.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/f64-const.js.wasm-no-tls-context wasm.yaml/wasm/self-test/test_LowLevelBinary_string.js.wasm-no-call-ic microbenchmarks/asmjs_bool_bug.js.default wasm.yaml/wasm/function-tests/loop-mult.js.wasm-eager-jettison wasm.yaml/wasm/js-api/wasm-to-wasm-bad-signature.js.wasm-no-call-ic wasm.yaml/wasm/fuzz/memory.js.wasm-no-tls-context wasm.yaml/wasm/js-api/element.js.default-wasm wasm.yaml/wasm/js-api/export-void-is-undef.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/element-data.js.default-wasm microbenchmarks/typed-array-subarray.js.ftl-no-cjit-no-put-stack-validate stress/put-direct-index-broken-2.js.ftl-eager-no-cjit microbenchmarks/asmjs_bool_bug.js.ftl-no-cjit-validate-sampling-profiler wasm.yaml/wasm/function-tests/context-switch.js.default-wasm wasm.yaml/wasm/function-tests/br-if-loop-less-than.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/exceptions.js.wasm-no-tls-context wasm.yaml/wasm/js-api/Module-compile.js.default-wasm wasm.yaml/wasm/js-api/web-assembly-compile-parallel.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/rotr.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/trap-load.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/grow-memory-4.js.wasm-no-call-ic wasm.yaml/wasm/js-api/Module.imports.js.wasm-no-tls-context microbenchmarks/typed-array-subarray.js.ftl-no-cjit-b3o1 microbenchmarks/asmjs_bool_bug.js.dfg-maximal-flush-validate-no-cjit wasm.yaml/wasm/fuzz/export-function.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/memory-grow.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/i32-const.js.wasm-eager-jettison wasm.yaml/wasm/self-test/test_LowLevelBinary_uint8.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/f64-const.js.default-wasm wasm.yaml/wasm/self-test/test_LowLevelBinary_varuint32.js.wasm-eager-jettison wasm.yaml/wasm/js-api/global-error.js.wasm-eager-jettison wasm.yaml/wasm/js-api/Module.exports.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/memory-section-and-import.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/self-test/test_LowLevelBinary_uint24.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/table-basic-2.js.wasm-no-call-ic wasm.yaml/wasm/js-api/Instance.imports.exports.unicode.js.wasm-no-tls-context microbenchmarks/typed-array-get-set-by-val-profiling.js.no-llint wasm.yaml/wasm/function-tests/trap-store-2.js.default-wasm wasm.yaml/wasm/js-api/test_Data.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/grow-memory.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/global-external-init-from-import.js.default-wasm jsc-layout-tests.yaml/js/script-tests/regress-155776.js.layout-ftl-eager-no-cjit microbenchmarks/typed-array-get-set-by-val-profiling.js.no-cjit-validate-phases stress/typedarray-slice.js.dfg-eager stress/typed-array-byte-offset.js.ftl-eager-no-cjit wasm.yaml/wasm/function-tests/br-as-return.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/Instance.imports.exports.unicode.js.wasm-eager-jettison microbenchmarks/typed-array-get-set-by-val-profiling.js.ftl-no-cjit-no-put-stack-validate wasm.yaml/wasm/function-tests/exceptions.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/Module.customSection.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/br-as-return.js.wasm-no-tls-context microbenchmarks/asmjs_bool_bug.js.ftl-eager-no-cjit microbenchmarks/typed-array-subarray.js.ftl-eager-no-cjit wasm.yaml/wasm/function-tests/loop-sum.js.wasm-no-call-ic wasm.yaml/wasm/self-test/test_LowLevelBinary_uint16.js.default-wasm wasm.yaml/wasm/function-tests/shr-s.js.wasm-no-call-ic wasm.yaml/wasm/js-api/Instance.imports.exports.unicode.js.default-wasm stress/typedarray-slice.js.no-cjit-validate-phases wasm.yaml/wasm/js-api/web-assembly-instantiate.js.wasm-no-tls-context wasm.yaml/wasm/js-api/test_Start.js.wasm-no-tls-context wasm.yaml/wasm/js-api/element.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/grow-memory-2.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/grow-memory-3.js.default-wasm microbenchmarks/asmjs_bool_bug.js.ftl-no-cjit-no-inline-validate wasm.yaml/wasm/function-tests/context-switch.js.wasm-no-call-ic stress/jsc-read.js.ftl-no-cjit-b3o1 wasm.yaml/wasm/js-api/wasm-to-wasm.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/self-test/test_LowLevelBinary_encode.js.wasm-no-tls-context microbenchmarks/typed-array-get-set-by-val-profiling.js.dfg-eager wasm.yaml/wasm/self-test/test_LowLevelBinary_string.js.wasm-no-cjit-yes-tls-context stress/typedarray-slice.js.no-ftl wasm.yaml/wasm/function-tests/br-if-loop-less-than.js.wasm-eager-jettison wasm.yaml/wasm/js-api/unique-signature.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/wrapper-function.js.wasm-no-call-ic wasm.yaml/wasm/js-api/global-internal-init-from-import.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/loop-sum.js.default-wasm wasm.yaml/wasm/function-tests/br-if-as-return.js.wasm-no-call-ic wasm.yaml/wasm/js-api/unique-signature.js.wasm-no-tls-context wasm.yaml/wasm/js-api/Module.imports.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/trap-load.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/memory-alignment.js.wasm-no-tls-context microbenchmarks/asmjs_bool_bug.js.dfg-eager wasm.yaml/wasm/function-tests/many-args-tail-call-sp-restored.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/exceptions.js.default-wasm stress/typedarray-slice.js.ftl-eager-no-cjit wasm.yaml/wasm/self-test/test_LowLevelBinary_uint24.js.default-wasm wasm.yaml/wasm/js-api/export-arity.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/shr-u.js.wasm-no-call-ic stress/put-direct-index-broken-2.js.ftl-no-cjit-no-put-stack-validate wasm.yaml/wasm/js-api/global-external-init-from-import.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/rotr.js.wasm-no-call-ic wasm.yaml/wasm/js-api/global-mutate.js.wasm-no-cjit-yes-tls-context microbenchmarks/typed-array-subarray.js.dfg-eager wasm.yaml/wasm/function-tests/memory-section-and-import.js.default-wasm wasm.yaml/wasm/function-tests/stack-overflow.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/f32-const.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/many-args-tail-call-sp-restored.js.wasm-no-tls-context stress/put-direct-index-broken-2.js.ftl-no-cjit-no-inline-validate microbenchmarks/typed-array-get-set-by-val-profiling.js.no-cjit-collect-continuously wasm.yaml/wasm/function-tests/ret5.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/shr-s.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/shl.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/global-external-init-from-import.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/brTableAsIf.js.default-wasm wasm.yaml/wasm/function-tests/stack-trace.js.default-wasm wasm.yaml/wasm/function-tests/grow-memory-3.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/memory-grow-invalid.js.default-wasm wasm.yaml/wasm/function-tests/trap-store.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/grow-memory-4.js.default-wasm wasm.yaml/wasm/js-api/global-error.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/brTableWithLoop.js.wasm-eager-jettison wasm.yaml/wasm/js-api/Module.exports.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/table-basic.js.default-wasm wasm.yaml/wasm/function-tests/grow-memory-2.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/lowExecutableMemory/imports-oom.js.default-wasm wasm.yaml/wasm/function-tests/f32-const.js.wasm-eager-jettison microbenchmarks/typed-array-get-set-by-val-profiling.js.ftl-eager stress/typedarray-slice.js.dfg-eager-no-cjit-validate wasm.yaml/wasm/function-tests/rotl.js.wasm-eager-jettison stress/put-direct-index-broken-2.js.dfg-maximal-flush-validate-no-cjit wasm.yaml/wasm/js-api/memory-grow.js.wasm-no-cjit-yes-tls-context stress/typed-array-byte-offset.js.no-cjit-collect-continuously wasm.yaml/wasm/js-api/web-assembly-instantiate.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.wasm-eager-jettison wasm.yaml/wasm/self-test/test_LowLevelBinary_encode.js.default-wasm wasm.yaml/wasm/js-api/web-assembly-instantiate-parallel.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/wrapper-function.js.wasm-no-tls-context stress/typed-array-byte-offset.js.ftl-no-cjit-small-pool wasm.yaml/wasm/js-api/call-indirect.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/br-if-loop-less-than.js.default-wasm wasm.yaml/wasm/js-api/test_Instance.js.wasm-no-cjit-yes-tls-context stress/put-direct-index-broken-2.js.default wasm.yaml/wasm/function-tests/i32-load8-s.js.default-wasm wasm.yaml/wasm/js-api/global-internal-init-from-import.js.default-wasm wasm.yaml/wasm/js-api/dont-mmap-zero-byte-memory.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/basic-element.js.wasm-eager-jettison wasm.yaml/wasm/js-api/globals-export.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/memory-section-and-import.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/stack-overflow.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/memory-section-and-import.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/jsapi.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/self-test/test_BuilderWebAssembly.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/load-offset.js.wasm-no-tls-context wasm.yaml/wasm/js-api/test_Instance.js.wasm-no-tls-context wasm.yaml/wasm/js-api/test_Start.js.wasm-eager-jettison wasm.yaml/wasm/js-api/wasm-to-wasm-bad-signature.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/dead-call.js.wasm-no-tls-context wasm.yaml/wasm/self-test/test_LowLevelBinary_varuint32.js.wasm-no-tls-context wasm.yaml/wasm/self-test/test_LowLevelBinary_uint32.js.default-wasm wasm.yaml/wasm/self-test/test_LowLevelBinary_uint8.js.wasm-no-tls-context wasm.yaml/wasm/js-api/web-assembly-instantiate-parallel.js.wasm-eager-jettison microbenchmarks/typed-array-subarray.js.ftl-eager microbenchmarks/typed-array-get-set-by-val-profiling.js.ftl-no-cjit-b3o1 wasm.yaml/wasm/js-api/Module-compile.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/dead-call.js.default-wasm wasm.yaml/wasm/self-test/test_LowLevelBinary_uint16.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/trap-load.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/stack-overflow.js.default-wasm stress/jsc-read.js.ftl-no-cjit-no-put-stack-validate stress/put-direct-index-broken-2.js.no-ftl wasm.yaml/wasm/js-api/wasm-to-wasm.js.wasm-no-call-ic wasm.yaml/wasm/self-test/test_LowLevelBinary_varint32.js.default-wasm wasm.yaml/wasm/function-tests/many-args-tail-call-sp-restored.js.default-wasm wasm.yaml/wasm/function-tests/stack-trace.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/ret5.js.wasm-no-call-ic wasm.yaml/wasm/js-api/call-indirect.js.wasm-eager-jettison stress/typedarray-slice.js.ftl-no-cjit-validate-sampling-profiler wasm.yaml/wasm/js-api/export-arity.js.wasm-no-tls-context wasm.yaml/wasm/js-api/element-data.js.wasm-no-cjit-yes-tls-context microbenchmarks/typed-array-subarray.js.no-cjit-collect-continuously wasm.yaml/wasm/js-api/memory-grow.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/memory-import-and-grow.js.wasm-eager-jettison microbenchmarks/typed-array-get-set-by-val-profiling.js.dfg-eager-no-cjit-validate wasm.yaml/wasm/js-api/test_Instance.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/i32-load.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/i32-const.js.default-wasm wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/br-as-return.js.default-wasm wasm.yaml/wasm/function-tests/shr-u.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/trap-load-2.js.default-wasm wasm.yaml/wasm/js-api/table.js.wasm-no-call-ic stress/put-direct-index-broken-2.js.no-cjit-validate-phases microbenchmarks/typed-array-subarray.js.dfg-eager-no-cjit-validate wasm.yaml/wasm/function-tests/memory-alignment.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/dead-call.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/basic-element.js.default-wasm wasm.yaml/wasm/js-api/element-data.js.wasm-no-call-ic wasm.yaml/wasm/js-api/extension-MemoryMode.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/web-assembly-instantiate.js.default-wasm wasm.yaml/wasm/js-api/web-assembly-function.js.wasm-eager-jettison wasm.yaml/wasm/self-test/test_LowLevelBinary_uint32.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/extension-MemoryMode.js.default-wasm wasm.yaml/wasm/function-tests/trap-after-cross-instance-call.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/function-import-return-value.js.wasm-no-call-ic wasm.yaml/wasm/js-api/web-assembly-compile-parallel.js.default-wasm wasm.yaml/wasm/function-tests/br-if-loop-less-than.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/memory-alignment.js.default-wasm wasm.yaml/wasm/function-tests/br-if-as-return.js.wasm-eager-jettison microbenchmarks/asmjs_bool_bug.js.no-ftl jsc-layout-tests.yaml/js/script-tests/regress-155776.js.layout-no-ftl wasm.yaml/wasm/function-tests/add-12.js.wasm-no-tls-context microbenchmarks/asmjs_bool_bug.js.no-llint wasm.yaml/wasm/function-tests/factorial.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/br-table-as-return.js.wasm-eager-jettison wasm.yaml/wasm/js-api/global-error.js.wasm-no-call-ic wasm.yaml/wasm/js-api/test_memory.js.default-wasm wasm.yaml/wasm/js-api/Module-compile.js.wasm-no-call-ic wasm.yaml/wasm/js-api/test_memory_constructor.js.wasm-no-tls-context wasm.yaml/wasm/self-test/test_LowLevelBinary_varuint32.js.default-wasm wasm.yaml/wasm/function-tests/context-switch.js.wasm-eager-jettison wasm.yaml/wasm/self-test/test_LowLevelBinary_uint8.js.default-wasm wasm.yaml/wasm/function-tests/memory-access-past-4gib.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/self-test/test_LowLevelBinary_uint16.js.wasm-no-cjit-yes-tls-context stress/put-direct-index-broken-2.js.no-cjit-collect-continuously wasm.yaml/wasm/function-tests/table-basic.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/i32-load.js.default-wasm stress/jsc-read.js.ftl-eager-no-cjit-b3o1 microbenchmarks/asmjs_bool_bug.js.ftl-eager wasm.yaml/wasm/function-tests/shr-u.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/unique-signature.js.default-wasm stress/typed-array-byte-offset.js.no-cjit-validate-phases wasm.yaml/wasm/function-tests/memory-grow-invalid.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/dead-call.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/element.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/trap-store-2.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/br-table-as-return.js.default-wasm wasm.yaml/wasm/self-test/test_LowLevelBinary_string.js.wasm-eager-jettison wasm.yaml/wasm/self-test/test_LowLevelBinary_varuint1.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/validate.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/self-test/test_LowLevelBinary_uint8.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/br-table-as-return.js.wasm-no-call-ic wasm.yaml/wasm/self-test/test_LowLevelBinary_varuint32.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/i32-load.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/add-12.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/i32-load8-s.js.wasm-eager-jettison wasm.yaml/wasm/js-api/validate.js.wasm-eager-jettison wasm.yaml/wasm/js-api/table.js.default-wasm wasm.yaml/wasm/js-api/Module.customSection.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/br-if-as-return.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/globals-import.js.wasm-eager-jettison wasm.yaml/wasm/js-api/global-internal-init-from-import.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/invalid-duplicate-export.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/br-as-return.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/brTableManyValues.js.wasm-no-call-ic wasm.yaml/wasm/js-api/global-error.js.default-wasm wasm.yaml/wasm/js-api/export-void-is-undef.js.wasm-no-tls-context wasm.yaml/wasm/js-api/Module.imports.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/if-no-else-non-void.js.default-wasm wasm.yaml/wasm/function-tests/f32-const.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/i32-load.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/shl.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/stack-trace.js.wasm-no-tls-context stress/jsc-read.js.ftl-no-cjit-small-pool wasm.yaml/wasm/js-api/globals-export.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/brTableWithLoop.js.wasm-no-tls-context wasm.yaml/wasm/self-test/test_LowLevelBinary_varuint1.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/rotl.js.default-wasm wasm.yaml/wasm/self-test/test_LowLevelBinary_varuint1.js.default-wasm wasm.yaml/wasm/self-test/test_LowLevelBinary_encode.js.wasm-eager-jettison wasm.yaml/wasm/self-test/test_LowLevelBinary_varint32.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/self-test/test_BuilderWebAssembly.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/self-test/test_LowLevelBinary_string.js.wasm-no-tls-context microbenchmarks/typed-array-subarray.js.ftl-no-cjit-small-pool wasm.yaml/wasm/function-tests/table-basic-2.js.default-wasm wasm.yaml/wasm/function-tests/memory-alignment.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/brTableAsIf.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/loop-mult.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.default-wasm wasm.yaml/wasm/js-api/global-external-init-from-import.js.wasm-eager-jettison wasm.yaml/wasm/js-api/Module-compile.js.wasm-no-cjit-yes-tls-context microbenchmarks/asmjs_bool_bug.js.no-cjit-validate-phases wasm.yaml/wasm/function-tests/float-sub.js.default-wasm wasm.yaml/wasm/function-tests/shl.js.wasm-eager-jettison wasm.yaml/wasm/js-api/test_memory_constructor.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/invalid-duplicate-export.js.wasm-no-tls-context stress/jsc-read.js.no-ftl wasm.yaml/wasm/js-api/web-assembly-compile-parallel.js.wasm-no-tls-context microbenchmarks/asmjs_bool_bug.js.ftl-no-cjit-no-put-stack-validate stress/typed-array-byte-offset.js.ftl-eager wasm.yaml/wasm/function-tests/dead-call.js.wasm-no-call-ic wasm.yaml/wasm/js-api/test_memory_constructor.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/float-sub.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/br-if-as-return.js.default-wasm wasm.yaml/wasm/function-tests/nameSection.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/rotr.js.wasm-eager-jettison wasm.yaml/wasm/self-test/test_LowLevelBinary_string.js.default-wasm microbenchmarks/typed-array-subarray.js.no-llint wasm.yaml/wasm/js-api/dont-mmap-zero-byte-memory.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/trap-after-cross-instance-call.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/table-basic-2.js.wasm-no-tls-context wasm.yaml/wasm/self-test/test_LowLevelBinary_uint8.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/memory-section-and-import.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/loop-sum.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/memory-import-and-grow.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/i32-load8-s.js.wasm-no-cjit-yes-tls-context microbenchmarks/typed-array-subarray.js.ftl-no-cjit-validate-sampling-profiler wasm.yaml/wasm/function-tests/struct.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/stack-trace.js.wasm-no-call-ic wasm.yaml/wasm/js-api/test_Data.js.wasm-no-call-ic stress/put-direct-index-broken-2.js.ftl-no-cjit-validate-sampling-profiler wasm.yaml/wasm/function-tests/trap-store-2.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/trap-load-2.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/export-arity.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/grow-memory-3.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/wasm-to-wasm.js.default-wasm wasm.yaml/wasm/js-api/dont-mmap-zero-byte-memory.js.wasm-eager-jettison microbenchmarks/asmjs_bool_bug.js.ftl-no-cjit-b3o1 wasm.yaml/wasm/self-test/test_LowLevelBinary_varint32.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/table-basic-2.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/web-assembly-function.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/function-import-return-value.js.wasm-eager-jettison wasm.yaml/wasm/fuzz/memory.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/Module.imports.js.default-wasm wasm.yaml/wasm/function-tests/trap-load.js.default-wasm wasm.yaml/wasm/js-api/test_memory.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/brTableWithLoop.js.default-wasm stress/jsc-read.js.default wasm.yaml/wasm/js-api/globals-export.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/trap-after-cross-instance-call.js.wasm-eager-jettison wasm.yaml/wasm/js-api/Instance.imports.exports.unicode.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/exceptions.js.wasm-eager-jettison stress/typedarray-slice.js.no-llint wasm.yaml/wasm/function-tests/stack-overflow.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/trap-store-2.js.wasm-no-cjit-yes-tls-context stress/jsc-read.js.dfg-eager microbenchmarks/typed-array-subarray.js.ftl-no-cjit-no-inline-validate wasm.yaml/wasm/self-test/test_BuilderWebAssembly.js.wasm-no-call-ic wasm.yaml/wasm/js-api/dont-mmap-zero-byte-memory.js.default-wasm wasm.yaml/wasm/spec-tests/jsapi.js.wasm-eager-jettison stress/typedarray-slice.js.no-cjit-collect-continuously wasm.yaml/wasm/function-tests/loop-sum.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/memory-import-and-grow.js.wasm-no-tls-context wasm.yaml/wasm/js-api/test_Start.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/export-void-is-undef.js.wasm-eager-jettison wasm.yaml/wasm/js-api/Module-compile.js.wasm-eager-jettison wasm.yaml/wasm/self-test/test_LowLevelBinary_uint24.js.wasm-eager-jettison wasm.yaml/wasm/js-api/global-mutate.js.default-wasm wasm.yaml/wasm/js-api/test_memory_constructor.js.default-wasm wasm.yaml/wasm/function-tests/brTableAsIf.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/if-no-else-non-void.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/many-arguments-to-function.js.default-wasm wasm.yaml/wasm/js-api/wasm-to-wasm.js.wasm-eager-jettison wasm.yaml/wasm/self-test/test_LowLevelBinary_uint16.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/trap-load-2.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/many-arguments-to-function.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/memory-access-past-4gib.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/shr-s.js.default-wasm stress/put-direct-index-broken-2.js.ftl-no-cjit-small-pool wasm.yaml/wasm/js-api/globals-import.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/web-assembly-compile-parallel.js.wasm-eager-jettison microbenchmarks/typed-array-subarray.js.dfg-maximal-flush-validate-no-cjit wasm.yaml/wasm/function-tests/memory-access-past-4gib.js.wasm-no-tls-context wasm.yaml/wasm/js-api/wasm-to-wasm-bad-signature.js.default-wasm wasm.yaml/wasm/js-api/validate.js.wasm-no-call-ic wasm.yaml/wasm/js-api/web-assembly-function.js.default-wasm wasm.yaml/wasm/self-test/test_LowLevelBinary_uint24.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/test_memory.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/grow-memory-2.js.default-wasm stress/typedarray-slice.js.default wasm.yaml/wasm/function-tests/trap-store.js.default-wasm wasm.yaml/wasm/js-api/unique-signature.js.wasm-no-call-ic stress/typed-array-byte-offset.js.dfg-eager-no-cjit-validate wasm.yaml/wasm/js-api/web-assembly-function.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/test_Module.js.wasm-no-call-ic wasm.yaml/wasm/fuzz/export-function.js.wasm-no-call-ic wasm.yaml/wasm/js-api/test_Module.js.default-wasm wasm.yaml/wasm/function-tests/shr-u.js.wasm-eager-jettison wasm.yaml/wasm/js-api/element-data.js.wasm-no-tls-context wasm.yaml/wasm/js-api/wrapper-function.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/grow-memory-4.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/brTableManyValues.js.wasm-eager-jettison wasm.yaml/wasm/fuzz/export-function.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/stack-overflow.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/ret5.js.default-wasm wasm.yaml/wasm/function-tests/shl.js.default-wasm wasm.yaml/wasm/js-api/web-assembly-instantiate.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/add-12.js.wasm-no-call-ic microbenchmarks/typed-array-get-set-by-val-profiling.js.ftl-no-cjit-validate-sampling-profiler wasm.yaml/wasm/self-test/test_LowLevelBinary_uint32.js.wasm-no-call-ic stress/typedarray-slice.js.ftl-no-cjit-small-pool wasm.yaml/wasm/self-test/test_LowLevelBinary_encode.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/struct.js.default-wasm wasm.yaml/wasm/js-api/global-internal-init-from-import.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/grow-memory-3.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/grow-memory-2.js.wasm-eager-jettison stress/typed-array-byte-offset.js.ftl-eager-no-cjit-b3o1 wasm.yaml/wasm/js-api/export-arity.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/loop-mult.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/trap-store.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/memory-import-and-grow.js.default-wasm microbenchmarks/typed-array-get-set-by-val-profiling.js.default wasm.yaml/wasm/js-api/extension-MemoryMode.js.wasm-no-tls-context wasm.yaml/wasm/function-tests/many-arguments-to-function.js.wasm-eager-jettison wasm.yaml/wasm/js-api/test_memory.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/js-api/test_Module.js.wasm-no-tls-context stress/typed-array-byte-offset.js.ftl-no-cjit-validate-sampling-profiler stress/jsc-read.js.no-cjit-collect-continuously wasm.yaml/wasm/self-test/test_LowLevelBinary_varuint1.js.wasm-no-tls-context wasm.yaml/wasm/self-test/test_LowLevelBinary_varint32.js.wasm-no-tls-context wasm.yaml/wasm/self-test/test_LowLevelBinary_encode.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/trap-store.js.wasm-no-call-ic
Build Bot
Comment 21 2017-07-23 20:46:08 PDT
Comment on attachment 316250 [details] the patch Attachment 316250 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4176025 Number of test failures exceeded the failure limit.
Build Bot
Comment 22 2017-07-23 20:46:09 PDT
Created attachment 316258 [details] Archive of layout-test-results from ews115 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Build Bot
Comment 23 2017-07-23 20:56:00 PDT
Comment on attachment 316250 [details] the patch Attachment 316250 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/4176032 Number of test failures exceeded the failure limit.
Build Bot
Comment 24 2017-07-23 20:56:02 PDT
Created attachment 316260 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.5
Filip Pizlo
Comment 25 2017-07-24 20:59:10 PDT
Created attachment 316349 [details] passing more tests I'm going to have to fix some things about how we do destructors so that we can easily destruct JSWebAssemblyMemory eagerly.
Filip Pizlo
Comment 26 2017-07-25 19:01:25 PDT
Created attachment 316418 [details] rebased Also started getting the wasm memory to do what I want. Now I just have to glue the bmalloc and GC support for wasm memory into WasmMemory.cpp.
Filip Pizlo
Comment 27 2017-07-25 20:24:41 PDT
Created attachment 316424 [details] getting closer
Filip Pizlo
Comment 28 2017-07-25 20:57:25 PDT
Created attachment 316427 [details] maybe it's done now
Filip Pizlo
Comment 29 2017-07-25 21:30:10 PDT
Created attachment 316431 [details] it compiles!
Filip Pizlo
Comment 30 2017-07-27 11:47:40 PDT
Created attachment 316560 [details] it runs wasmbench!
Filip Pizlo
Comment 31 2017-07-27 14:19:46 PDT
Created attachment 316572 [details] came up with a more principled way of handling null
Filip Pizlo
Comment 32 2017-07-27 15:12:32 PDT
Created attachment 316574 [details] fixed more wasm memory bugs
Filip Pizlo
Comment 33 2017-07-27 17:12:28 PDT
Created attachment 316583 [details] it has a chance of passing JSC tests Fixed handling of WasmMemory OOM
Filip Pizlo
Comment 34 2017-07-27 18:14:15 PDT
Created attachment 316587 [details] passes JSC tests
Filip Pizlo
Comment 35 2017-07-27 18:32:33 PDT
Created attachment 316593 [details] rebased I'm only now trying to build WebCore...
Build Bot
Comment 36 2017-07-27 18:34:55 PDT
Attachment 316593 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/heap/Subspace.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.cpp:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/Gigacage.cpp:35: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Gigacage.h:54: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:55: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:68: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:69: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/Heap.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/VM.cpp:425: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 18 in 88 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 37 2017-07-27 19:18:35 PDT
Created attachment 316602 [details] fix 32-bit
Build Bot
Comment 38 2017-07-27 19:21:38 PDT
Attachment 316602 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/heap/Subspace.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.cpp:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/Gigacage.cpp:35: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Gigacage.h:54: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:55: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:68: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:69: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/Heap.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/VM.cpp:425: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 18 in 88 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 39 2017-07-27 19:27:13 PDT
Created attachment 316603 [details] try to fix gcc build
Build Bot
Comment 40 2017-07-27 19:29:08 PDT
Attachment 316603 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/heap/Subspace.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.cpp:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/Gigacage.cpp:35: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Gigacage.h:54: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:55: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:68: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:69: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/Heap.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/VM.cpp:425: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 18 in 88 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 41 2017-07-27 19:34:27 PDT
Created attachment 316604 [details] fix more builds
Build Bot
Comment 42 2017-07-27 19:36:47 PDT
Attachment 316604 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/heap/Subspace.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.cpp:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/Gigacage.cpp:35: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Gigacage.h:54: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:55: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:68: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:69: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/Heap.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/VM.cpp:425: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 18 in 88 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 43 2017-07-27 20:01:20 PDT
Created attachment 316606 [details] the patch Fixed more build issues. Referenced a bunch of other bugs.
Build Bot
Comment 44 2017-07-27 20:03:58 PDT
Attachment 316606 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/heap/Subspace.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.cpp:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/Gigacage.cpp:35: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Gigacage.h:54: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:55: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:68: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:69: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/Heap.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/VM.cpp:425: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 18 in 90 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 45 2017-07-27 20:26:26 PDT
Created attachment 316607 [details] more fixes
Build Bot
Comment 46 2017-07-27 20:29:57 PDT
Attachment 316607 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/heap/Subspace.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.cpp:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/Gigacage.cpp:35: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Gigacage.h:54: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:55: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:68: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:69: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/Heap.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/VM.cpp:425: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 18 in 91 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 47 2017-07-27 20:30:54 PDT
Build Bot
Comment 48 2017-07-27 20:33:32 PDT
Attachment 316608 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/heap/Subspace.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.cpp:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/Gigacage.cpp:35: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Gigacage.h:54: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:55: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:68: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:69: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/Heap.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/VM.cpp:425: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 18 in 91 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 49 2017-07-27 21:16:19 PDT
Created attachment 316612 [details] more Trying to fix windows.
Build Bot
Comment 50 2017-07-27 21:19:58 PDT
Attachment 316612 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/heap/Subspace.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.cpp:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/Gigacage.cpp:35: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WTF/wtf/Gigacage.h:54: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:55: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:68: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:69: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/Heap.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/VM.cpp:425: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 18 in 91 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 51 2017-07-27 22:37:55 PDT
Comment on attachment 316612 [details] more Attachment 316612 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4200372 New failing tests: fast/canvas/webgpu/webgpu-dispatch.html
Build Bot
Comment 52 2017-07-27 22:37:57 PDT
Created attachment 316617 [details] Archive of layout-test-results from ews106 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Saam Barati
Comment 53 2017-07-28 01:45:21 PDT
Comment on attachment 316612 [details] more View in context: https://bugs.webkit.org/attachment.cgi?id=316612&action=review some brief comments. I'll look more later > Source/JavaScriptCore/wasm/WasmMemory.cpp:298 > + if (mprotect(fastMemory + initialBytes, Memory::fastMappedBytes() - initialBytes, PROT_NONE)) { Do we want an abstraction for this since you use the OSAllocator abstraction above? > Source/JavaScriptCore/wasm/WasmMemory.cpp:367 > + bool done = tryAndGC( nit: I'm not sure what you mean by "done" here. Maybe there is a better name? > Source/JavaScriptCore/wasm/WasmMemory.cpp:381 > + memset(newMemory, 0, desiredSize - m_size); This looks wrong. I think you want (newMemory + m_size) as the ptr to memset. If this didn't fail any tests, perhaps you should add one. > Source/bmalloc/bmalloc/Gigacage.cpp:35 > +// OOPS! (need bug title) oops > Source/bmalloc/bmalloc/Gigacage.cpp:78 > +#endif // BOS(DARWIN) && BCPU(X86_64) Should be: // GIGACAGE_ENABLED > Source/bmalloc/bmalloc/Gigacage.h:35 > +#define GIGACAGE_RUNWAY (16lu * 1024 * 1024 * 1024) This is unused.
Filip Pizlo
Comment 54 2017-07-28 13:25:45 PDT
(In reply to Build Bot from comment #51) > Comment on attachment 316612 [details] > more > > Attachment 316612 [details] did not pass mac-wk2-ews (mac-wk2): > Output: http://webkit-queues.webkit.org/results/4200372 > > New failing tests: > fast/canvas/webgpu/webgpu-dispatch.html This is real. Investigating...
Filip Pizlo
Comment 55 2017-07-28 15:40:57 PDT
Created attachment 316680 [details] fixed metal
Build Bot
Comment 56 2017-07-28 15:44:01 PDT
Attachment 316680 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/heap/Subspace.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.cpp:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/Gigacage.cpp:35: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WTF/wtf/Gigacage.h:56: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:57: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:65: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:70: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:71: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/Heap.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/VM.cpp:425: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 19 in 93 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 57 2017-07-28 15:48:42 PDT
For the curious among us, what is the goal / purpose of the gigacage?
Build Bot
Comment 58 2017-07-28 16:59:56 PDT
Comment on attachment 316680 [details] fixed metal Attachment 316680 [details] did not pass jsc-ews (mac): Output: http://webkit-queues.webkit.org/results/4204896 New failing tests: wasm.yaml/wasm/stress/oom.js.wasm-no-call-ic
Build Bot
Comment 59 2017-07-28 18:19:24 PDT
Comment on attachment 316680 [details] fixed metal Attachment 316680 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/4205247 New failing tests: fast/cookies/cookie-averse-document.html
Build Bot
Comment 60 2017-07-28 18:19:26 PDT
Created attachment 316689 [details] Archive of layout-test-results from ews100 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Myles C. Maxfield
Comment 61 2017-07-29 00:11:06 PDT
“Gigacage” was the prison they held Professor X in, right?
Filip Pizlo
Comment 62 2017-07-29 12:14:31 PDT
Created attachment 316721 [details] fix some issues I think I fixed the crash, the JSC test failure, and the Windows build error.
Build Bot
Comment 63 2017-07-29 12:17:58 PDT
Attachment 316721 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/heap/Subspace.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.cpp:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WTF/wtf/Gigacage.h:53: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:56: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:57: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:65: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:70: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:71: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/Heap.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/VM.cpp:425: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 20 in 93 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 64 2017-07-29 12:24:50 PDT
(In reply to Saam Barati from comment #53) > Comment on attachment 316612 [details] > more > > View in context: > https://bugs.webkit.org/attachment.cgi?id=316612&action=review > > some brief comments. I'll look more later > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:298 > > + if (mprotect(fastMemory + initialBytes, Memory::fastMappedBytes() - initialBytes, PROT_NONE)) { > > Do we want an abstraction for this since you use the OSAllocator abstraction > above? Nah. I think that abstractions for this are overkill unless we actually want to port this code to non-POSIX platforms. > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:367 > > + bool done = tryAndGC( > > nit: I'm not sure what you mean by "done" here. Maybe there is a better name? I changed it to success. > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:381 > > + memset(newMemory, 0, desiredSize - m_size); > > This looks wrong. I think you want (newMemory + m_size) as the ptr to > memset. If this didn't fail any tests, perhaps you should add one. Fixed. I'll look into the test. > > > Source/bmalloc/bmalloc/Gigacage.cpp:35 > > +// OOPS! (need bug title) > > oops Fixed. > > > Source/bmalloc/bmalloc/Gigacage.cpp:78 > > +#endif // BOS(DARWIN) && BCPU(X86_64) > > Should be: // GIGACAGE_ENABLED Fixed. > > > Source/bmalloc/bmalloc/Gigacage.h:35 > > +#define GIGACAGE_RUNWAY (16lu * 1024 * 1024 * 1024) > > This is unused. Oh right, I meant to use that. I'll fix that.
Filip Pizlo
Comment 65 2017-07-29 12:52:43 PDT
Created attachment 316723 [details] the patch Try EWS again now that I fixed the build. Also, this includes some fixes to address Saam's feedback.
Build Bot
Comment 66 2017-07-29 12:56:38 PDT
Attachment 316723 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/heap/Subspace.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.cpp:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WTF/wtf/Gigacage.h:53: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:56: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:57: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:65: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:70: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:71: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/Heap.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/VM.cpp:425: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 20 in 93 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 67 2017-07-29 13:29:42 PDT
Created attachment 316726 [details] trial patch Attempting to add a test for the WasmMemory bug. Attempting to fix windows. This has the WasmMemory bug and the test - I want to see if EWS catches it.
Build Bot
Comment 68 2017-07-29 13:33:03 PDT
Attachment 316726 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:381: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/heap/Subspace.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.cpp:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WTF/wtf/Gigacage.h:53: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:56: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:57: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:65: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:70: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:71: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/Heap.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/VM.cpp:425: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 21 in 93 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 69 2017-07-29 13:57:14 PDT
Created attachment 316727 [details] the patch Fixed Windows. Confirmed that the WasmMemory test works.
Build Bot
Comment 70 2017-07-29 13:59:29 PDT
Attachment 316727 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/heap/Subspace.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.cpp:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WTF/wtf/Gigacage.h:53: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:56: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:57: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:65: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:70: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:71: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/Heap.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/VM.cpp:425: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 20 in 93 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 71 2017-07-29 14:47:52 PDT
Created attachment 316728 [details] the patch Fixing more build failures.
Build Bot
Comment 72 2017-07-29 14:50:22 PDT
Attachment 316728 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/heap/Subspace.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.cpp:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WTF/wtf/Gigacage.h:53: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:56: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:57: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:65: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:70: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:71: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/Heap.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/VM.cpp:425: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 20 in 93 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 73 2017-07-29 15:41:41 PDT
Created attachment 316729 [details] the patch More fixes...
Build Bot
Comment 74 2017-07-29 15:44:36 PDT
Attachment 316729 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/heap/Subspace.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.cpp:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WTF/wtf/Gigacage.h:53: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:56: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:57: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:65: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:70: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:71: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/Heap.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/VM.cpp:425: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 20 in 93 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 75 2017-07-29 17:23:06 PDT
Created attachment 316730 [details] maybe this will do it
Build Bot
Comment 76 2017-07-29 17:26:55 PDT
Attachment 316730 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/heap/Subspace.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.cpp:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WTF/wtf/Gigacage.h:53: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:56: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:57: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:65: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:70: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:71: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/Heap.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/VM.cpp:425: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 20 in 93 files If any of these errors are false positives, please file a bug against check-webkit-style.
JF Bastien
Comment 77 2017-07-31 10:19:16 PDT
Comment on attachment 316730 [details] maybe this will do it View in context: https://bugs.webkit.org/attachment.cgi?id=316730&action=review Only looked at WebAssembly for now. The WebAssembly fast memory pre-allocation was critical in getting a fair number of pages when in constrained memory, otherwise our address space was too fragmented and there weren't holes big enough. It looks like gigacage makes that a non-issue because it gets its own huge slab of virtual memory? This loses VM_TAG_FOR_WEBASSEMBLY_MEMORY. Is that desirable? > Source/JavaScriptCore/wasm/WasmMemory.cpp:146 > + MemoryResult::Kind tryAllocatePhysicalBytes(size_t bytes) This name is misleading because it doesn't actually allocate anything. It checks whether the allocation is an acceptable size, and then accounts for those bytes. > Source/JavaScriptCore/wasm/WasmMemory.cpp:282 > return nullptr; Here, I initially thought this actually allocated! It just checks whether it's likely to work. This doesn't take fast memory into account though: it only logs the initial bytes, and doesn't take virtual memory pressure into account. Is that on purpose, since gigacage just has virtual space and it's not a resource worth accounting for anymore? > Source/JavaScriptCore/wasm/WasmMemory.cpp:296 > + OSAllocator::commit(fastMemory, initialBytes, true, false); Can you make those not bools but enums while you're changing code? > Source/JavaScriptCore/wasm/WasmMemory.cpp:301 > + } Why is protection of the tail needed here? It should already be PROT_NONE. > Source/JavaScriptCore/wasm/WasmMemory.cpp:303 > + memset(fastMemory, 0, initialBytes); Why memset? It should already be zero. Is it to force early commit? > Source/JavaScriptCore/wasm/WasmMemory.cpp:313 > + void* slowMemory = Gigacage::tryAlignedMalloc(WTF::pageSize(), initialBytes); The previous code first tried allocating maximum if it was provided, and only if that failed did it allocate initial. That's kind of the expectation when providing a maximum. Why drop this? > Source/JavaScriptCore/wasm/WasmMemory.cpp:318 > + memset(slowMemory, 0, initialBytes); Ditto, is this needed? > Source/JavaScriptCore/wasm/WasmMemory.cpp:334 > + } I guess memset is needed because gigacage doesn't guarantee zero pages? Old code used to zero out stuff here. > Source/JavaScriptCore/wasm/WasmMemory.cpp:379 > + void* newMemory = Gigacage::tryAlignedMalloc(WTF::pageSize(), desiredSize); Can you realloc first? > Source/JavaScriptCore/wasm/WasmMemory.cpp:380 > + memcpy(newMemory, m_memory, m_size); newMemory can be nullptr here.
JF Bastien
Comment 78 2017-07-31 10:20:05 PDT
Comment on attachment 316730 [details] maybe this will do it View in context: https://bugs.webkit.org/attachment.cgi?id=316730&action=review Only looked at WebAssembly for now. The WebAssembly fast memory pre-allocation was critical in getting a fair number of pages when in constrained memory, otherwise our address space was too fragmented and there weren't holes big enough. It looks like gigacage makes that a non-issue because it gets its own huge slab of virtual memory? This loses VM_TAG_FOR_WEBASSEMBLY_MEMORY. Is that desirable? > Source/JavaScriptCore/wasm/WasmMemory.cpp:146 > + MemoryResult::Kind tryAllocatePhysicalBytes(size_t bytes) This name is misleading because it doesn't actually allocate anything. It checks whether the allocation is an acceptable size, and then accounts for those bytes. > Source/JavaScriptCore/wasm/WasmMemory.cpp:282 > return nullptr; Here, I initially thought this actually allocated! It just checks whether it's likely to work. This doesn't take fast memory into account though: it only logs the initial bytes, and doesn't take virtual memory pressure into account. Is that on purpose, since gigacage just has virtual space and it's not a resource worth accounting for anymore? > Source/JavaScriptCore/wasm/WasmMemory.cpp:296 > + OSAllocator::commit(fastMemory, initialBytes, true, false); Can you make those not bools but enums while you're changing code? > Source/JavaScriptCore/wasm/WasmMemory.cpp:301 > + } Why is protection of the tail needed here? It should already be PROT_NONE. > Source/JavaScriptCore/wasm/WasmMemory.cpp:303 > + memset(fastMemory, 0, initialBytes); Why memset? It should already be zero. Is it to force early commit? > Source/JavaScriptCore/wasm/WasmMemory.cpp:313 > + void* slowMemory = Gigacage::tryAlignedMalloc(WTF::pageSize(), initialBytes); The previous code first tried allocating maximum if it was provided, and only if that failed did it allocate initial. That's kind of the expectation when providing a maximum. Why drop this? > Source/JavaScriptCore/wasm/WasmMemory.cpp:318 > + memset(slowMemory, 0, initialBytes); Ditto, is this needed? > Source/JavaScriptCore/wasm/WasmMemory.cpp:334 > + } I guess memset is needed because gigacage doesn't guarantee zero pages? Old code used to zero out stuff here. > Source/JavaScriptCore/wasm/WasmMemory.cpp:379 > + void* newMemory = Gigacage::tryAlignedMalloc(WTF::pageSize(), desiredSize); Can you realloc first? > Source/JavaScriptCore/wasm/WasmMemory.cpp:380 > + memcpy(newMemory, m_memory, m_size); newMemory can be nullptr here.
Filip Pizlo
Comment 79 2017-07-31 12:51:44 PDT
(In reply to JF Bastien from comment #77) > Comment on attachment 316730 [details] > maybe this will do it > > View in context: > https://bugs.webkit.org/attachment.cgi?id=316730&action=review > > Only looked at WebAssembly for now. > > The WebAssembly fast memory pre-allocation was critical in getting a fair > number of pages when in constrained memory, otherwise our address space was > too fragmented and there weren't holes big enough. It looks like gigacage > makes that a non-issue because it gets its own huge slab of virtual memory? Yeah. > > This loses VM_TAG_FOR_WEBASSEMBLY_MEMORY. Is that desirable? It's not desirable. :-( But, if we're in the Gigacage then the memory is already tagged. > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:146 > > + MemoryResult::Kind tryAllocatePhysicalBytes(size_t bytes) > > This name is misleading because it doesn't actually allocate anything. It > checks whether the allocation is an acceptable size, and then accounts for > those bytes. Yeah. Do you have a better name? > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:282 > > return nullptr; > > Here, I initially thought this actually allocated! It just checks whether > it's likely to work. > > This doesn't take fast memory into account though: it only logs the initial > bytes, and doesn't take virtual memory pressure into account. Is that on > purpose, since gigacage just has virtual space and it's not a resource worth > accounting for anymore? We're failing because we know that there isn't enough physical memory for the allocation. If we succeed here, we may still fail when allocating virtual memory. > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:296 > > + OSAllocator::commit(fastMemory, initialBytes, true, false); > > Can you make those not bools but enums while you're changing code? I'm not changing OSAllocator. :-) But I'll name the booleans here. > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:301 > > + } > > Why is protection of the tail needed here? It should already be PROT_NONE. Nope. Bmalloc keeps unused memory around as PROT_READ|PROT_WRITE but decommitted. > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:303 > > + memset(fastMemory, 0, initialBytes); > > Why memset? It should already be zero. Is it to force early commit? Nope. Bmalloc won't necessarily zero memory you return to it. > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:313 > > + void* slowMemory = Gigacage::tryAlignedMalloc(WTF::pageSize(), initialBytes); > > The previous code first tried allocating maximum if it was provided, and > only if that failed did it allocate initial. That's kind of the expectation > when providing a maximum. Why drop this? No evidence that we needed it. > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:318 > > + memset(slowMemory, 0, initialBytes); > > Ditto, is this needed? Yeah. > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:334 > > + } > > I guess memset is needed because gigacage doesn't guarantee zero pages? Old > code used to zero out stuff here. Yeah. > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:379 > > + void* newMemory = Gigacage::tryAlignedMalloc(WTF::pageSize(), desiredSize); > > Can you realloc first? Bmalloc doesn't have realloc. > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:380 > > + memcpy(newMemory, m_memory, m_size); > > newMemory can be nullptr here. Right! Oops! Fixed.
JF Bastien
Comment 80 2017-07-31 12:59:24 PDT
> > > Source/JavaScriptCore/wasm/WasmMemory.cpp:146 > > > + MemoryResult::Kind tryAllocatePhysicalBytes(size_t bytes) > > > > This name is misleading because it doesn't actually allocate anything. It > > checks whether the allocation is an acceptable size, and then accounts for > > those bytes. > > Yeah. Do you have a better name? tryAddingPhysicalMemoryToBookkeeping ? willThisFitAndIfSoPutItOnTheBooks ? > > > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:282 > > > return nullptr; > > > > Here, I initially thought this actually allocated! It just checks whether > > it's likely to work. > > > > This doesn't take fast memory into account though: it only logs the initial > > bytes, and doesn't take virtual memory pressure into account. Is that on > > purpose, since gigacage just has virtual space and it's not a resource worth > > accounting for anymore? > > We're failing because we know that there isn't enough physical memory for > the allocation. > > If we succeed here, we may still fail when allocating virtual memory. What I'm saying is: old code used to account for actually allocated memory, as well as virtual allocation. IIUC all the "dead" pages that we allocate just for PROT_NONE never get counted in the updated code. > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:313 > > > + void* slowMemory = Gigacage::tryAlignedMalloc(WTF::pageSize(), initialBytes); > > > > The previous code first tried allocating maximum if it was provided, and > > only if that failed did it allocate initial. That's kind of the expectation > > when providing a maximum. Why drop this? > > No evidence that we needed it. > > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:379 > > > + void* newMemory = Gigacage::tryAlignedMalloc(WTF::pageSize(), desiredSize); > > > > Can you realloc first? > > Bmalloc doesn't have realloc. The above two things are potential regressions if users use max, or if they don't but just grow. We indeed have zero data that people do it, but I'm wary of regressing this. I guess we have time to pick u on such cases though.
Mark Lam
Comment 81 2017-07-31 17:22:05 PDT
Comment on attachment 316730 [details] maybe this will do it View in context: https://bugs.webkit.org/attachment.cgi?id=316730&action=review Some comments for now (I'm still reading and digesting the patch but need to stop for a bit). > Source/JavaScriptCore/jsc.cpp:2315 > + JSObject* result = JSUint8Array::create(exec->vm(), structure, WTFMove(content)); vm is already available. No need to use exec->vm(). > Source/JavaScriptCore/API/JSTypedArray.cpp:307 > - > + Please undo. > Source/JavaScriptCore/wasm/WasmFaultSignalHandler.cpp:124 > - > + Please undo. > Source/JavaScriptCore/wasm/js/JSWebAssemblyMemory.cpp:49 > - > + Please undo. > Source/JavaScriptCore/wasm/js/JSWebAssemblyMemory.cpp:77 > - > + Please undo. > Source/bmalloc/ChangeLog:8 > + This adds a mechanism for managing multiple isolated heaps in bmalloc. For now, these isoheaps have a Can you define the term "isoheaps"? Is it short for "isolated heaps"? Is this a term of art? In scientific parlance, "iso" typically means "same" or "equal". The first time I read "isoheap", I was thinking same heap (as in no isolation). But I think you mean the opposite here i.e. isolated heaps. > Source/bmalloc/bmalloc/Allocator.cpp:188 > - > + Please undo. > Source/bmalloc/bmalloc/Heap.cpp:267 > - > + Please undo. > Source/bmalloc/bmalloc/Heap.cpp:526 > - > + Please undo. > Source/bmalloc/bmalloc/HeapKind.h:35 > +static const unsigned numHeaps = 2; Let's make this a constexpr. Sometimes, lldb may decide that static const actually needs linkage to memory storage (and I don't always understand why). Also, I think in webkit-style, we typically prefer to have this spelled out as numberOfHeaps. > Source/bmalloc/bmalloc/PerHeapKind.h:49 > + return reinterpret_cast<T*>(&m_memory)[i]; I think this should be "*reinterpret_cast<T*>(&m_memory[i])". I looked at http://en.cppreference.com/w/cpp/types/aligned_storage, and I don't see anything that says that it guarantees that sizeof(Memory) == sizeof(T) i.e. it may be that sizeof(Memory) >= sizeof(T). I presume we used std::aligned_storage in the first place because we want each element to be properly aligned. Hence, using &m_memory[i] is the right thing to do. > Source/bmalloc/bmalloc/PerHeapKind.h:54 > + return reinterpret_cast<T*>(&m_memory)[i]; Ditto. Same as above.
Filip Pizlo
Comment 82 2017-08-01 10:59:01 PDT
(In reply to JF Bastien from comment #80) > > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:146 > > > > + MemoryResult::Kind tryAllocatePhysicalBytes(size_t bytes) > > > > > > This name is misleading because it doesn't actually allocate anything. It > > > checks whether the allocation is an acceptable size, and then accounts for > > > those bytes. > > > > Yeah. Do you have a better name? > > tryAddingPhysicalMemoryToBookkeeping ? > willThisFitAndIfSoPutItOnTheBooks ? Meh... those twist my tongue too much. > > > > > > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:282 > > > > return nullptr; > > > > > > Here, I initially thought this actually allocated! It just checks whether > > > it's likely to work. > > > > > > This doesn't take fast memory into account though: it only logs the initial > > > bytes, and doesn't take virtual memory pressure into account. Is that on > > > purpose, since gigacage just has virtual space and it's not a resource worth > > > accounting for anymore? > > > > We're failing because we know that there isn't enough physical memory for > > the allocation. > > > > If we succeed here, we may still fail when allocating virtual memory. > > What I'm saying is: old code used to account for actually allocated memory, > as well as virtual allocation. IIUC all the "dead" pages that we allocate > just for PROT_NONE never get counted in the updated code. This code accounts for both also. > > > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:313 > > > > + void* slowMemory = Gigacage::tryAlignedMalloc(WTF::pageSize(), initialBytes); > > > > > > The previous code first tried allocating maximum if it was provided, and > > > only if that failed did it allocate initial. That's kind of the expectation > > > when providing a maximum. Why drop this? > > > > No evidence that we needed it. > > > > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:379 > > > > + void* newMemory = Gigacage::tryAlignedMalloc(WTF::pageSize(), desiredSize); > > > > > > Can you realloc first? > > > > Bmalloc doesn't have realloc. > > The above two things are potential regressions if users use max, or if they > don't but just grow. We indeed have zero data that people do it, but I'm > wary of regressing this. I guess we have time to pick u on such cases though. We can fix it if that becomes a problem. :-)
Filip Pizlo
Comment 83 2017-08-01 11:02:25 PDT
(In reply to Mark Lam from comment #81) > Comment on attachment 316730 [details] > maybe this will do it > > View in context: > https://bugs.webkit.org/attachment.cgi?id=316730&action=review > > Some comments for now (I'm still reading and digesting the patch but need to > stop for a bit). > > > Source/JavaScriptCore/jsc.cpp:2315 > > + JSObject* result = JSUint8Array::create(exec->vm(), structure, WTFMove(content)); > > vm is already available. No need to use exec->vm(). Fixed. > > > Source/JavaScriptCore/API/JSTypedArray.cpp:307 > > - > > + > > Please undo. Fixed. > > > Source/JavaScriptCore/wasm/WasmFaultSignalHandler.cpp:124 > > - > > + > > Please undo. Fixed. > > > Source/JavaScriptCore/wasm/js/JSWebAssemblyMemory.cpp:49 > > - > > + > > Please undo. Fixed. > > > Source/JavaScriptCore/wasm/js/JSWebAssemblyMemory.cpp:77 > > - > > + > > Please undo. Fixed. > > > Source/bmalloc/ChangeLog:8 > > + This adds a mechanism for managing multiple isolated heaps in bmalloc. For now, these isoheaps have a > > Can you define the term "isoheaps"? Is it short for "isolated heaps"? Is > this a term of art? In scientific parlance, "iso" typically means "same" or > "equal". The first time I read "isoheap", I was thinking same heap (as in > no isolation). But I think you mean the opposite here i.e. isolated heaps. Fixed. It's "isolated heaps". > > > Source/bmalloc/bmalloc/Allocator.cpp:188 > > - > > + > > Please undo. Fixed. > > > Source/bmalloc/bmalloc/Heap.cpp:267 > > - > > + > > Please undo. Fixed. > > > Source/bmalloc/bmalloc/Heap.cpp:526 > > - > > + > > Please undo. Fixed. > > > Source/bmalloc/bmalloc/HeapKind.h:35 > > +static const unsigned numHeaps = 2; > > Let's make this a constexpr. Sometimes, lldb may decide that static const > actually needs linkage to memory storage (and I don't always understand why). > > Also, I think in webkit-style, we typically prefer to have this spelled out > as numberOfHeaps. Fixed. > > > Source/bmalloc/bmalloc/PerHeapKind.h:49 > > + return reinterpret_cast<T*>(&m_memory)[i]; > > I think this should be "*reinterpret_cast<T*>(&m_memory[i])". I looked at > http://en.cppreference.com/w/cpp/types/aligned_storage, and I don't see > anything that says that it guarantees that sizeof(Memory) == sizeof(T) i.e. > it may be that sizeof(Memory) >= sizeof(T). I presume we used > std::aligned_storage in the first place because we want each element to be > properly aligned. Hence, using &m_memory[i] is the right thing to do. > > > Source/bmalloc/bmalloc/PerHeapKind.h:54 > > + return reinterpret_cast<T*>(&m_memory)[i]; > > Ditto. Same as above. Fixed and fixed.
Radar WebKit Bug Importer
Comment 84 2017-08-01 11:08:15 PDT
Filip Pizlo
Comment 85 2017-08-01 12:16:36 PDT
Created attachment 316879 [details] rebased Also applied more review feedback.
Build Bot
Comment 86 2017-08-01 12:19:12 PDT
Attachment 316879 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:374: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/heap/Subspace.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.cpp:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WTF/wtf/Gigacage.h:53: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:56: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:57: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:65: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:70: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:71: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/Heap.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/VM.cpp:425: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 20 in 94 files If any of these errors are false positives, please file a bug against check-webkit-style.
Mark Lam
Comment 87 2017-08-01 12:22:46 PDT
Comment on attachment 316730 [details] maybe this will do it View in context: https://bugs.webkit.org/attachment.cgi?id=316730&action=review I see that you've rebased with new code. I'll just post my additional comments here, and then resume reviewing on the new patch. > Source/bmalloc/bmalloc/Gigacage.cpp:45 > + Callback() > + { > + } nit: you can just put this on 1 line: Callback() { } or Callback() = default; > Source/bmalloc/bmalloc/Gigacage.h:35 > +#define GIGACAGE_RUNWAY (16lu * 1024 * 1024 * 1024) Why not just say "#define GIGACAGE_RUNWAY GIGACAGE_SIZE"? Isn't it always intended to be the same size? > Source/bmalloc/bmalloc/Heap.cpp:59 > + if (m_kind == HeapKind::Gigacage) > + Gigacage::ensureGigacage(); You can put this snippit in the #if GIGACAGE_ENABLED below. It would have been a call to an empty function anyway. > Source/bmalloc/bmalloc/Heap.cpp:138 > + pthread_set_qos_class_self_np(PerProcess<Scavenger>::get()->requestedScavengerThreadQOSClass(), 0); You can use PerProcess<Scavenger>::getFastCase() here because the Heap constructor already ensured that the Scavenger is created (line 69 above). > Source/bmalloc/bmalloc/Heap.cpp:-542 > - scheduleScavenger(size); This is a change in logic. I think Michael did a lot of memory work to discover that this call is needed (in https://trac.webkit.org/r217456). Unless you have a good reason for removing it, I suggest retaining the call. However, given that we can deallocate different AllocationKinds, maybe it only make sense to scavenge if allocationKind == Physical? > Source/bmalloc/bmalloc/Heap.h:119 > + HeapKind m_kind; nit: not a big deal, but you can move m_kind down to next to m_isGrowing for better compaction. > Source/bmalloc/bmalloc/bmalloc.h:68 > +// Returns null of failure "Returns null for failure"?
Filip Pizlo
Comment 88 2017-08-01 12:31:00 PDT
(In reply to Mark Lam from comment #87) > Comment on attachment 316730 [details] > maybe this will do it > > View in context: > https://bugs.webkit.org/attachment.cgi?id=316730&action=review > > I see that you've rebased with new code. I'll just post my additional > comments here, and then resume reviewing on the new patch. > > > Source/bmalloc/bmalloc/Gigacage.cpp:45 > > + Callback() > > + { > > + } > > nit: you can just put this on 1 line: Callback() { } or Callback() = default; Fixed. > > > Source/bmalloc/bmalloc/Gigacage.h:35 > > +#define GIGACAGE_RUNWAY (16lu * 1024 * 1024 * 1024) > > Why not just say "#define GIGACAGE_RUNWAY GIGACAGE_SIZE"? Isn't it always > intended to be the same size? They're not the same size. GIGACAGE_RUNWAY is a function of array element size (8 bytes) and maximum index size (usually 2^31). GIGACAGE_SIZE is supposed to be 64GB, not 16GB. But I just realized that in this patch it's actually 1024GB. I'll fix!! > > > Source/bmalloc/bmalloc/Heap.cpp:59 > > + if (m_kind == HeapKind::Gigacage) > > + Gigacage::ensureGigacage(); > > You can put this snippit in the #if GIGACAGE_ENABLED below. It would have > been a call to an empty function anyway. Fixed. > > > Source/bmalloc/bmalloc/Heap.cpp:138 > > + pthread_set_qos_class_self_np(PerProcess<Scavenger>::get()->requestedScavengerThreadQOSClass(), 0); > > You can use PerProcess<Scavenger>::getFastCase() here because the Heap > constructor already ensured that the Scavenger is created (line 69 above). Fixed. > > > Source/bmalloc/bmalloc/Heap.cpp:-542 > > - scheduleScavenger(size); > > This is a change in logic. I think Michael did a lot of memory work to > discover that this call is needed (in https://trac.webkit.org/r217456). > Unless you have a good reason for removing it, I suggest retaining the call. > > However, given that we can deallocate different AllocationKinds, maybe it > only make sense to scavenge if allocationKind == Physical? You're right, I didn't mean to remove the call to scheduleScavenger(). In fact, Geoff had specifically suggested keeping the scheduleScavenger() call in place, even for Physical. I'll do that! > > > Source/bmalloc/bmalloc/Heap.h:119 > > + HeapKind m_kind; > > nit: not a big deal, but you can move m_kind down to next to m_isGrowing for > better compaction. I'll keep it here for now, just 'cause it makes most logical sense to come first. > > > Source/bmalloc/bmalloc/bmalloc.h:68 > > +// Returns null of failure > > "Returns null for failure"? Fixed.
Filip Pizlo
Comment 89 2017-08-01 13:01:24 PDT
(In reply to Filip Pizlo from comment #88) > (In reply to Mark Lam from comment #87) > > Comment on attachment 316730 [details] > > maybe this will do it > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=316730&action=review > > > > I see that you've rebased with new code. I'll just post my additional > > comments here, and then resume reviewing on the new patch. > > > > > Source/bmalloc/bmalloc/Gigacage.cpp:45 > > > + Callback() > > > + { > > > + } > > > > nit: you can just put this on 1 line: Callback() { } or Callback() = default; > > Fixed. > > > > > > Source/bmalloc/bmalloc/Gigacage.h:35 > > > +#define GIGACAGE_RUNWAY (16lu * 1024 * 1024 * 1024) > > > > Why not just say "#define GIGACAGE_RUNWAY GIGACAGE_SIZE"? Isn't it always > > intended to be the same size? > > They're not the same size. GIGACAGE_RUNWAY is a function of array element > size (8 bytes) and maximum index size (usually 2^31). > > GIGACAGE_SIZE is supposed to be 64GB, not 16GB. But I just realized that in > this patch it's actually 1024GB. I'll fix!! > > > > > > Source/bmalloc/bmalloc/Heap.cpp:59 > > > + if (m_kind == HeapKind::Gigacage) > > > + Gigacage::ensureGigacage(); > > > > You can put this snippit in the #if GIGACAGE_ENABLED below. It would have > > been a call to an empty function anyway. > > Fixed. Turns out that's wrong. ensureGigacage() is needed for usingGigacage() to return the correct value. > > > > > > Source/bmalloc/bmalloc/Heap.cpp:138 > > > + pthread_set_qos_class_self_np(PerProcess<Scavenger>::get()->requestedScavengerThreadQOSClass(), 0); > > > > You can use PerProcess<Scavenger>::getFastCase() here because the Heap > > constructor already ensured that the Scavenger is created (line 69 above). > > Fixed. > > > > > > Source/bmalloc/bmalloc/Heap.cpp:-542 > > > - scheduleScavenger(size); > > > > This is a change in logic. I think Michael did a lot of memory work to > > discover that this call is needed (in https://trac.webkit.org/r217456). > > Unless you have a good reason for removing it, I suggest retaining the call. > > > > However, given that we can deallocate different AllocationKinds, maybe it > > only make sense to scavenge if allocationKind == Physical? > > You're right, I didn't mean to remove the call to scheduleScavenger(). In > fact, Geoff had specifically suggested keeping the scheduleScavenger() call > in place, even for Physical. I'll do that! > > > > > > Source/bmalloc/bmalloc/Heap.h:119 > > > + HeapKind m_kind; > > > > nit: not a big deal, but you can move m_kind down to next to m_isGrowing for > > better compaction. > > I'll keep it here for now, just 'cause it makes most logical sense to come > first. > > > > > > Source/bmalloc/bmalloc/bmalloc.h:68 > > > +// Returns null of failure > > > > "Returns null for failure"? > > Fixed.
Filip Pizlo
Comment 90 2017-08-01 13:13:07 PDT
Created attachment 316887 [details] with more fixes Fixed the GIGACAGE_MASK so that it's 64GB for reals.
Build Bot
Comment 91 2017-08-01 13:15:27 PDT
Attachment 316887 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:374: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190: More than one command on the same line [whitespace/newline] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/heap/Subspace.h:63: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.cpp:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/Gigacage.cpp:36: g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WTF/wtf/Gigacage.h:53: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:56: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:57: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:65: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:70: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/Gigacage.h:71: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/bmalloc/bmalloc/Heap.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/VM.cpp:425: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 20 in 94 files If any of these errors are false positives, please file a bug against check-webkit-style.
Mark Lam
Comment 92 2017-08-01 13:31:33 PDT
Comment on attachment 316879 [details] rebased View in context: https://bugs.webkit.org/attachment.cgi?id=316879&action=review I think some of the style checker complaints are valid. Would be nice to fix the valid (especially trivial low risk) ones before landing. I have a few more files to review, but just want to post what I have so far first. > Source/JavaScriptCore/heap/SubspaceInlines.h:57 > + allocator.forEachBlock(func); You've changed this from forEachNotEmptyBlock to forEachBlock. Is this intentional, or is this a copy-paste error? > Source/JavaScriptCore/runtime/ArrayBuffer.cpp:221 > - contents.tryAllocate(byteLength, 1, ArrayBufferContents::ZeroInitialize); > + contents.tryAllocate(byteLength, 1, ArrayBufferContents::DontInitialize); I think you did this intentionally because createInternal() will full the bits with data below. But just in case, I thought I'd flag it. > Source/JavaScriptCore/runtime/VM.h:761 > - > + Please undo. > Source/bmalloc/bmalloc/Gigacage.cpp:94 > + Callbacks& callbacks = *PerProcess<Callbacks>::get(); > + std::unique_lock<StaticMutex> lock(PerProcess<Callbacks>::mutex()); > + for (Callback& callback : callbacks.callbacks) > + callback.function(callback.argument); > + callbacks.callbacks.shrink(0); I don't see the part where you actually disable the gigacage and clear g_gigacageBasePtr.
Filip Pizlo
Comment 93 2017-08-01 13:35:10 PDT
(In reply to Mark Lam from comment #92) > Comment on attachment 316879 [details] > rebased > > View in context: > https://bugs.webkit.org/attachment.cgi?id=316879&action=review > > I think some of the style checker complaints are valid. Would be nice to > fix the valid (especially trivial low risk) ones before landing. > > I have a few more files to review, but just want to post what I have so far > first. > > > Source/JavaScriptCore/heap/SubspaceInlines.h:57 > > + allocator.forEachBlock(func); > > You've changed this from forEachNotEmptyBlock to forEachBlock. Is this > intentional, or is this a copy-paste error? It's an error. Fixed. It would have been correctness-neutral (it's always OK to also visit empty blocks) but it might have perf implications. > > > Source/JavaScriptCore/runtime/ArrayBuffer.cpp:221 > > - contents.tryAllocate(byteLength, 1, ArrayBufferContents::ZeroInitialize); > > + contents.tryAllocate(byteLength, 1, ArrayBufferContents::DontInitialize); > > I think you did this intentionally because createInternal() will full the > bits with data below. But just in case, I thought I'd flag it. Yeah, I think this is right. > > > Source/JavaScriptCore/runtime/VM.h:761 > > - > > + > > Please undo. Fixed. > > > Source/bmalloc/bmalloc/Gigacage.cpp:94 > > + Callbacks& callbacks = *PerProcess<Callbacks>::get(); > > + std::unique_lock<StaticMutex> lock(PerProcess<Callbacks>::mutex()); > > + for (Callback& callback : callbacks.callbacks) > > + callback.function(callback.argument); > > + callbacks.callbacks.shrink(0); > > I don't see the part where you actually disable the gigacage and clear > g_gigacageBasePtr. Fixed.
Build Bot
Comment 94 2017-08-01 14:59:31 PDT
Comment on attachment 316887 [details] with more fixes Attachment 316887 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/4235967 New failing tests: imported/blink/fast/dom/HTMLImageElement/image-sizes-viewport-with-template-parent-and-empty-sizes.html
Build Bot
Comment 95 2017-08-01 14:59:33 PDT
Created attachment 316900 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.5
Build Bot
Comment 96 2017-08-01 15:02:59 PDT
Comment on attachment 316887 [details] with more fixes Attachment 316887 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4235936 New failing tests: fast/dom/Window/HTMLBodyElement-window-eventListener-attributes.html
Build Bot
Comment 97 2017-08-01 15:03:01 PDT
Created attachment 316901 [details] Archive of layout-test-results from ews117 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Filip Pizlo
Comment 98 2017-08-01 17:22:05 PDT
(In reply to Build Bot from comment #97) > Created attachment 316901 [details] > Archive of layout-test-results from ews117 for mac-elcapitan > > The attached test failures were seen while running run-webkit-tests on the > mac-debug-ews. > Bot: ews117 Port: mac-elcapitan Platform: Mac OS X 10.11.6 These crashes look unrelated to my change. Looks like this is a regression in trunk.
Saam Barati
Comment 99 2017-08-01 17:34:03 PDT
Comment on attachment 316887 [details] with more fixes View in context: https://bugs.webkit.org/attachment.cgi?id=316887&action=review > Source/JavaScriptCore/dfg/DFGFixedButterflyAccessUncagingPhase.cpp:63 > + case GetByOffset: > + case PutByOffset: > + case GetGetterSetterByOffset: > + case GetArrayLength: > + case GetVectorLength: I'm not sure I understand why we can whitelist these. What happens if some attacker overwrote the butterfly pointer in the JSObject. This would just trust the value that's there? > Source/JavaScriptCore/dfg/DFGFixedButterflyAccessUncagingPhase.cpp:76 > + if (!readsOverlap(m_graph, node, Heap)) > + break; I'm not sure this matters in practice given how Clobberize is written today, but I'd argue that this should also consider writes into the Heap, not just reads.
Mark Lam
Comment 100 2017-08-01 17:35:46 PDT
Comment on attachment 316887 [details] with more fixes View in context: https://bugs.webkit.org/attachment.cgi?id=316887&action=review I don't understand how !readsOverlap(m_graph, node, Heap) means that we don't need to cage the read of the butterfly pointer. Can you explain? Everything else looks good to me. > Source/JavaScriptCore/dfg/DFGFixedButterflyAccessUncagingPhase.cpp:58 > + // Check if this is needed: This comment is misaligned. > Source/JavaScriptCore/heap/MarkedAllocator.cpp:108 > + RELEASE_ASSERT(block->subspace()->canTradeBlocksWith(m_subspace)); > + RELEASE_ASSERT(m_subspace->canTradeBlocksWith(block->subspace())); Do these really need to be RELEASE_ASSERTs? All we're doing here is confirming that no one broke findEmptyBlockToSteal(). I feel that a debug ASSERT would do.
Filip Pizlo
Comment 101 2017-08-01 17:42:35 PDT
(In reply to Mark Lam from comment #100) > Comment on attachment 316887 [details] > with more fixes > > View in context: > https://bugs.webkit.org/attachment.cgi?id=316887&action=review > > I don't understand how !readsOverlap(m_graph, node, Heap) means that we > don't need to cage the read of the butterfly pointer. Can you explain? We only need to do caging if we do indexes access to the butterfly. If a node does not read from the heap then it does not do indexed accesses to the butterfly. > > Everything else looks good to me. > > > Source/JavaScriptCore/dfg/DFGFixedButterflyAccessUncagingPhase.cpp:58 > > + // Check if this is needed: > > This comment is misaligned. I fixed it and turned it into a proper FIXME. > > > Source/JavaScriptCore/heap/MarkedAllocator.cpp:108 > > + RELEASE_ASSERT(block->subspace()->canTradeBlocksWith(m_subspace)); > > + RELEASE_ASSERT(m_subspace->canTradeBlocksWith(block->subspace())); > > Do these really need to be RELEASE_ASSERTs? All we're doing here is > confirming that no one broke findEmptyBlockToSteal(). I feel that a debug > ASSERT would do. If we get this wrong then bmalloc craps itself in horrible ways. Like, it often just deadlocks. So, I was being extra paranoid.
Filip Pizlo
Comment 102 2017-08-01 17:49:58 PDT
(In reply to Filip Pizlo from comment #101) > (In reply to Mark Lam from comment #100) > > Comment on attachment 316887 [details] > > with more fixes > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=316887&action=review > > > > I don't understand how !readsOverlap(m_graph, node, Heap) means that we > > don't need to cage the read of the butterfly pointer. Can you explain? > > We only need to do caging if we do indexes access to the butterfly. > > If a node does not read from the heap then it does not do indexed accesses > to the butterfly. But what if it writes instead of reads. I'm going to change this to accessesOverlap. > > > > > Everything else looks good to me. > > > > > Source/JavaScriptCore/dfg/DFGFixedButterflyAccessUncagingPhase.cpp:58 > > > + // Check if this is needed: > > > > This comment is misaligned. > > I fixed it and turned it into a proper FIXME. > > > > > > Source/JavaScriptCore/heap/MarkedAllocator.cpp:108 > > > + RELEASE_ASSERT(block->subspace()->canTradeBlocksWith(m_subspace)); > > > + RELEASE_ASSERT(m_subspace->canTradeBlocksWith(block->subspace())); > > > > Do these really need to be RELEASE_ASSERTs? All we're doing here is > > confirming that no one broke findEmptyBlockToSteal(). I feel that a debug > > ASSERT would do. > > If we get this wrong then bmalloc craps itself in horrible ways. Like, it > often just deadlocks. > > So, I was being extra paranoid.
Mark Lam
Comment 103 2017-08-01 18:29:48 PDT
Comment on attachment 316887 [details] with more fixes View in context: https://bugs.webkit.org/attachment.cgi?id=316887&action=review r=me with remaining issues resolved. > Source/JavaScriptCore/dfg/DFGFixedButterflyAccessUncagingPhase.cpp:75 > + if (!readsOverlap(m_graph, node, Heap)) We discussed offline that this should be accessOverlaps instead.
Filip Pizlo
Comment 104 2017-08-01 18:50:58 PDT
Saam Barati
Comment 105 2017-08-04 18:56:32 PDT
This patch regressed ARES-6 by 2% on Mac.
Saam Barati
Comment 106 2017-08-04 19:05:48 PDT
(In reply to Saam Barati from comment #105) > This patch regressed ARES-6 by 2% on Mac. I filed: https://bugs.webkit.org/show_bug.cgi?id=175234
Note You need to log in before you can comment on or make changes to this bug.