Bug 179528 – The memory consumption of DFG::BasicBlock can be easily reduced a bit

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Robin Morisset]

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.

[Robin Morisset]

Created attachment 326581 [details]
Patch

[Build Bot]

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.

[Saam Barati]

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.

[Robin Morisset]

Created attachment 326589 [details]
Patch for landing

[WebKit Commit Bot]

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

[Robin Morisset]

Created attachment 326592 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 326592 [details]
Patch for landing

Clearing flags on attachment: 326592

Committed r224689: <https://trac.webkit.org/changeset/224689>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35562082>