I see several ways of doing it: - Reordering a few fields to reduce padding brings sizeof(BasicBlock) from 3032 to 3016. - Making Operands a single vector (safe since we never change the number of arguments) instead of 2 reduces the size from 3016 to 2984, and halves the number of memory allocations when both inline buffer overflow, and avoids overflowing in some cases when we have just a bit more locals than 16, but don't have the full 8 arguments, or the reverse. As a bonus, it makes BasicBlocks more readable when printed in the debugger. This patch includes both of these changes (since the first one is fairly trivial). No performance change on some quick benchmarks (none was expected, since this just reduces memory pressure a bit) Another idea (for a possible further patch) would be to merge all five Operands in BasicBlock into a single Operand of a struct with 5 fields (since all Operands share the same number of locals and of arguments). This would reduce memory consumption and allocation more significantly, but would have a risk of harming memory locality depending on the access pattern. It would also be significantly more involved, since it would require fixing every location that uses one of these 5 fields.
Created attachment 326581 [details] Patch
Attachment 326581 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGBasicBlock.cpp:39: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGBasicBlock.cpp:40: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGBasicBlock.cpp:41: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGBasicBlock.cpp:46: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGBasicBlock.cpp:47: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGBasicBlock.cpp:48: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] Total errors found: 6 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 326581 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=326581&action=review r=me > Source/JavaScriptCore/bytecode/Operands.h:186 > { You can just write this as: operand(operand) = value > Source/JavaScriptCore/bytecode/Operands.h:252 > + size_t m_numArguments; I say make this unsigned. This probably won’t matter because of padding, but it might be useful if someone adds a field in the future.
Created attachment 326589 [details] Patch for landing
Comment on attachment 326589 [details] Patch for landing Rejecting attachment 326589 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: es/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/Objects-normal/x86_64/UnifiedSource28.o ** BUILD FAILED ** The following build commands failed: CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/Objects-normal/x86_64/UnifiedSource9.o /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/JavaScriptCore/unified-sources/UnifiedSource9.cpp normal x86_64 c++ com.apple.compilers.llvm.clang.1_0.compiler (1 failure) Full output: http://webkit-queues.webkit.org/results/5177748
Created attachment 326592 [details] Patch for landing
Comment on attachment 326592 [details] Patch for landing Clearing flags on attachment: 326592 Committed r224689: <https://trac.webkit.org/changeset/224689>
All reviewed patches have been landed. Closing bug.
<rdar://problem/35562082>