Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JSC] Sort opcodes to remove padding from metadata table #9927

Conversation

tadeuzagallo
Copy link
Member

@tadeuzagallo tadeuzagallo commented Feb 10, 2023

e2606b1

[JSC] Sort opcodes to remove padding from metadata table
https://bugs.webkit.org/show_bug.cgi?id=252055
rdar://105276503

Reviewed by Yusuke Suzuki.

Sort the opcodes by metadata alignment requirements. We start with 8-byte aligned,
followed by 4-byte, etc. The memory savings are minimal (too small to measure with
confidence), but they are also free.

* Source/JavaScriptCore/bytecode/BytecodeList.rb:
* Source/JavaScriptCore/bytecode/UnlinkedMetadataTable.cpp:
(JSC::UnlinkedMetadataTable::finalize):

Canonical link: https://commits.webkit.org/260193@main

4b30616

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ›  gtk
βœ… πŸ§ͺ webkitperl   πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk1 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  πŸ§ͺ jsc βœ… πŸ›  tv βœ… πŸ§ͺ mac-wk2 βœ… πŸ›  jsc-armv7
βœ… πŸ›  πŸ§ͺ jsc-arm64 βœ… πŸ›  tv-sim   πŸ§ͺ mac-AS-debug-wk2   πŸ§ͺ jsc-armv7-tests
βœ… πŸ›  watch βœ… πŸ§ͺ mac-wk2-stress βœ… πŸ›  jsc-mips
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch-sim βœ… πŸ§ͺ jsc-mips-tests

@tadeuzagallo tadeuzagallo requested a review from a team as a code owner February 10, 2023 16:07
@tadeuzagallo tadeuzagallo self-assigned this Feb 10, 2023
@tadeuzagallo tadeuzagallo added the JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. label Feb 10, 2023
Copy link
Member

@Constellation Constellation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me with GCC fix.

Comment on lines 35 to 40
#define JSC_ALIGNMENT_CHECK(size) static_assert(size <= UnlinkedMetadataTable::s_maxMetadataAlignment);
static_assert((UnlinkedMetadataTable::s_maxMetadataAlignment >=
#define JSC_ALIGNMENT_CHECK(size) size) && (size >=
FOR_EACH_BYTECODE_METADATA_ALIGNMENT(JSC_ALIGNMENT_CHECK)
#undef JSC_ALIGNMENT_CHECK
1));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like GCC is not happy with this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's probably because when computing the order I assumed that pointers were 8-byte aligned. I might just skip both new assertions on 32-bit since we will still align if necessary.

@tadeuzagallo tadeuzagallo force-pushed the eng/JSC-Sort-opcodes-to-remove-padding-from-metadata-table branch from 05710ee to 4b30616 Compare February 13, 2023 10:42
@tadeuzagallo tadeuzagallo added the merge-queue Applied to send a pull request to merge-queue label Feb 13, 2023
https://bugs.webkit.org/show_bug.cgi?id=252055
rdar://105276503

Reviewed by Yusuke Suzuki.

Sort the opcodes by metadata alignment requirements. We start with 8-byte aligned,
followed by 4-byte, etc. The memory savings are minimal (too small to measure with
confidence), but they are also free.

* Source/JavaScriptCore/bytecode/BytecodeList.rb:
* Source/JavaScriptCore/bytecode/UnlinkedMetadataTable.cpp:
(JSC::UnlinkedMetadataTable::finalize):

Canonical link: https://commits.webkit.org/260193@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/JSC-Sort-opcodes-to-remove-padding-from-metadata-table branch from 4b30616 to e2606b1 Compare February 13, 2023 12:29
@webkit-commit-queue
Copy link
Collaborator

Committed 260193@main (e2606b1): https://commits.webkit.org/260193@main

Reviewed commits have been landed. Closing PR #9927 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit e2606b1 into WebKit:main Feb 13, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Feb 13, 2023
@tadeuzagallo tadeuzagallo deleted the eng/JSC-Sort-opcodes-to-remove-padding-from-metadata-table branch May 17, 2023 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues.
Projects
None yet
4 participants