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] Do not use bytecode cache on $.agent worker threads #776

Merged
merged 0 commits into from May 23, 2022

Conversation

gezalore
Copy link
Contributor

@gezalore gezalore commented May 19, 2022

20ac310

[JSC] Do not use bytecode cache on $.agent worker threads

Patch by Geza Lore <glore@igalia.com > on 2022-05-23
https://bugs.webkit.org/show_bug.cgi?id=240642

Reviewed by Yusuke Suzuki.

Workers started via $.agent.start are not shut down in a synchronous
manner, and it is possible the main thread terminates the process while
a worker is writing its bytecode cache, which results in intermittent
test failures. As $.agent.start is only a rarely used testing facility,
we simply do not cache bytecode on these threads.

Also un-skip test on ARMv7 that used to fail because of this.

* Source/JavaScriptCore/jsc.cpp:
(Worker::isMain const):
(Worker::Worker):
(runJSC):
 * JSTests/stress/lars-sab-workers.js:

Canonical link: https://commits.webkit.org/250858@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294632 268f45cc-cd09-0410-ab3c-d52691b4dbfc

@gezalore gezalore added JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. WebKit Nightly Build labels May 19, 2022
@gezalore gezalore closed this May 19, 2022
@gezalore gezalore deleted the nobc-on-threads branch May 19, 2022 12:30
@gezalore gezalore restored the nobc-on-threads branch May 19, 2022 13:21
@gezalore gezalore reopened this May 19, 2022
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label May 19, 2022
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

Copy link
Contributor

@kmiller68 kmiller68 left a comment

Choose a reason for hiding this comment

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

Can we make it so other threads can read from the cache but not write? That way we can test the thread safety of using the caching system (at least partly).

@gezalore
Copy link
Contributor Author

@kmiller68 The agent threads execute different code from the main thread, so whatever the main thread writes to the cache is not what the agent threads would try to read, so you would never have a cache hit, so I don't see much point int trying to do that.

@gezalore
Copy link
Contributor Author

@kmiller68 What would you like me to do? Can we unblock this please?

@kmiller68 kmiller68 dismissed their stale review May 20, 2022 22:21

Sounds like my chance won’t help. Unblocking.

@Constellation Constellation added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels May 23, 2022
@webkit-early-warning-system webkit-early-warning-system merged commit 20ac310 into WebKit:main May 23, 2022
@webkit-early-warning-system
Copy link
Collaborator

Committed r294632 (250858@main): https://commits.webkit.org/250858@main

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

@webkit-early-warning-system webkit-early-warning-system removed the merge-queue Applied to send a pull request to merge-queue label May 23, 2022
@gezalore gezalore deleted the nobc-on-threads branch May 23, 2022 11:23
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