Closed Bug 1174319 Opened 4 years ago Closed 4 years ago
Remove the intermediate store buffer buffer
This would have been incredibly useful for inlining store buffer puts in the jits, but it turns out that's not actually that much of a bottleneck. I think we're not going to do this in the short term, so we can always re-add it easily enough if needed later.
Comment on attachment 8621842 [details] [diff] [review] remove_storebuffer_intermediate_buffer-v0.diff Never mind, this appears to dramatically increase our icache miss rate.
Comment on attachment 8621842 [details] [diff] [review] remove_storebuffer_intermediate_buffer-v0.diff I experimented with a few more variants and ended up retesting this a few more times. Turns out I just got two 3 sigma runs in a row in the wrong direction. After a few more runs, things actually come out even or lightly in favor of removing the intermediate buffer.
Comment on attachment 8621842 [details] [diff] [review] remove_storebuffer_intermediate_buffer-v0.diff Review of attachment 8621842 [details] [diff] [review]: ----------------------------------------------------------------- I'm kind of surprised by this, but if your measurements say it's OK then let's do it.
Attachment #8621842 - Flags: review?(jcoppeard) → review+
https://treeherder.mozilla.org/#/jobs?repo=try&revision=014e576eaa15 Notably M(oth) and SM(cgc) are passing now, which I think was a problem with my tip yesterday.
A regression/improvement was reported on AWFY: - slave: Win 8 32-bit (i7-4700HQ, browser) - mode: Ion Regression(s)/Improvement(s): - octane: -2.71% (regression) - octane: Richards: -3.17% (regression) - octane: zlib: -1.78% (regression) - ss: tagcloud: 24.29% (regression) - dromaeo: sunspider-string-validate-input: -5.84% (regression) - dromaeo: sunspider-bitops-nsieve-bits: -1.26% (regression) - dromaeo: sunspider-controlflow-recursive: -1.13% (regression) - dromaeo: sunspider-3d-raytrace: -2.04% (regression) - dromaeo: sunspider-bitops-bitwise-and: -1.93% (regression) - dromaeo: sunspider-access-nbody: -2.36% (regression) - dromaeo: sunspider-math-spectral-norm: -1.76% (regression) - dromaeo: sunspider-crypto-md5: -1.79% (regression) Recorded range: - http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=43e3ece52f55&tochange=4c3af36331e0 More details: http://arewefastyet.com/regressions/#/regression/1786178
I reproduced this between one before this patch and tip on zlib. Between one patch before and this however, I see no difference, so it's definitely not this patch. Perhaps the next, will try it next. Before -- Score (version 9): 59760 After -- Score (version 9): 59951
I think maybe I reproduced a regression in my MQ, because I see no difference between before and my current tip.
Backed out for now -- I don't have time to track this down while paying attention to work-week presentations.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Having the intermediate buffer be a single element makes construction of temporary RelocatablePtr much cheaper and simplifies this code tremendously, so I think we'll stick with that.
Status: REOPENED → RESOLVED
Closed: 4 years ago → 4 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.