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.
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.
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
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
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
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
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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
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.
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.
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
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.
(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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
(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.
> > > 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.
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.
(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. :-)
(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.
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.
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"?
(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.
(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.
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.
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.
(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.
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
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
(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.
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.
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.
(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.
(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.
2017-07-21 14:40 PDT, Filip Pizlo
2017-07-21 18:40 PDT, Filip Pizlo
2017-07-21 19:20 PDT, Filip Pizlo
2017-07-21 21:32 PDT, Filip Pizlo
2017-07-22 10:12 PDT, Filip Pizlo
2017-07-22 10:42 PDT, Filip Pizlo
2017-07-22 16:29 PDT, Filip Pizlo
2017-07-22 16:54 PDT, Filip Pizlo
2017-07-22 18:17 PDT, Filip Pizlo
2017-07-23 17:55 PDT, Filip Pizlo
2017-07-23 19:22 PDT, Filip Pizlo
2017-07-23 20:33 PDT, Build Bot
2017-07-23 20:38 PDT, Build Bot
2017-07-23 20:46 PDT, Build Bot
2017-07-23 20:56 PDT, Build Bot
2017-07-24 20:59 PDT, Filip Pizlo
2017-07-25 19:01 PDT, Filip Pizlo
2017-07-25 20:24 PDT, Filip Pizlo
2017-07-25 20:57 PDT, Filip Pizlo
2017-07-25 21:30 PDT, Filip Pizlo
2017-07-27 11:47 PDT, Filip Pizlo
2017-07-27 14:19 PDT, Filip Pizlo
2017-07-27 15:12 PDT, Filip Pizlo
2017-07-27 17:12 PDT, Filip Pizlo
2017-07-27 18:14 PDT, Filip Pizlo
2017-07-27 18:32 PDT, Filip Pizlo
2017-07-27 19:18 PDT, Filip Pizlo
2017-07-27 19:27 PDT, Filip Pizlo
2017-07-27 19:34 PDT, Filip Pizlo
2017-07-27 20:01 PDT, Filip Pizlo
2017-07-27 20:26 PDT, Filip Pizlo
2017-07-27 20:30 PDT, Filip Pizlo
2017-07-27 21:16 PDT, Filip Pizlo
2017-07-27 22:37 PDT, Build Bot
2017-07-28 15:40 PDT, Filip Pizlo
2017-07-28 18:19 PDT, Build Bot
2017-07-29 12:14 PDT, Filip Pizlo
2017-07-29 12:52 PDT, Filip Pizlo
2017-07-29 13:29 PDT, Filip Pizlo
2017-07-29 13:57 PDT, Filip Pizlo
2017-07-29 14:47 PDT, Filip Pizlo
2017-07-29 15:41 PDT, Filip Pizlo
2017-07-29 17:23 PDT, Filip Pizlo
2017-08-01 12:16 PDT, Filip Pizlo
2017-08-01 13:13 PDT, Filip Pizlo
buildbot: commit-queue-
2017-08-01 14:59 PDT, Build Bot
2017-08-01 15:03 PDT, Build Bot