Skip to content

Conversation

danleh
Copy link
Contributor

@danleh danleh commented Aug 14, 2025

We had discussed removing this workload a couple of months ago, but wanted to wait for more Wasm workloads to land. Now we can, I think. Disabling this also lowers the Wasm proportion of the total score.

Reasons: It's not realistic at all (it basically inserts elements in a HashSet ~8000 times; the two add instantiations account for ~75% of the CPU samples, see screenshot and symbol map), over-incentivizes inlining, and we have better workloads now. E.g., if we care about general datastructures implemented in Wasm, the sqlite workload is much larger, comes from an upstream benchmark suite, and exercises more diverse code.

image

We could also completely remove instead of disabling it, but I went for the conservative option for now.

We could also remove gcc-loops-wasm and quicksort-wasm, which are also quite microbenchmarky.

WDYT? @kmiller68 @eqrion @iainireland

  1. Disabling this fine? Remove it altogether?
  2. How about gcc-loops-wasm and quicksort-wasm?

Copy link

netlify bot commented Aug 14, 2025

Deploy Preview for webkit-jetstream-preview ready!

Name Link
🔨 Latest commit 9c7eced
🔍 Latest deploy log https://app.netlify.com/projects/webkit-jetstream-preview/deploys/689dc51ecaab8b00085f9f3c
😎 Deploy Preview https://deploy-preview-140--webkit-jetstream-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

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.

r=me

@eqrion
Copy link
Contributor

eqrion commented Aug 14, 2025

Removing this sounds good to me.

@danleh danleh merged commit b3e3f76 into WebKit:main Aug 14, 2025
9 of 10 checks passed
@danleh danleh deleted the disable-HashSet-wasm branch August 14, 2025 14:54
@danleh
Copy link
Contributor Author

danleh commented Aug 14, 2025

Set to disabled for now, in case we want to track this on internal CIs or so. Removing/cleanup later is easy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants