Closed Bug 1224038 Opened 5 years ago Closed 5 years ago
Use stable hashing for Object
Group Compartment::New Table
This one looks like it's going to be a bit complicated.
It passes tests locally. Try run link above.
Attachment #8688044 - Flags: review?(jcoppeard)
Comment on attachment 8688044 [details] [diff] [review] stable_ObjectGroupCompartmentNewTable-v0.diff Review of attachment 8688044 [details] [diff] [review]: ----------------------------------------------------------------- Great, much nicer.
Attachment #8688044 - Flags: review?(jcoppeard) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/f3d2ddb4979c9909e012adafd607fc4463e32e6b Bug 1224038 - Use stable hashing in ObjectGroupCompartment::NewTable; r=jonco
A regression/improvement was reported on AWFY: - slave: Mac OS X 10.10 32-bit (Mac Pro, shell) - mode: Ion Regression(s)/Improvement(s): - octane: CodeLoad: -1.38% (regression) - octane: Box2D: -7.58% (regression) - octane: Typescript: -7.21% (regression) Recorded range: - http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a621c6d49da1&tochange=f3d2ddb4979c More details: http://arewefastyet.com/regressions/#/regression/1799692
https://hg.mozilla.org/integration/mozilla-inbound/rev/cfc8f256cea03a2d930f4a65cbabcfd5652e7dcc Backout f3d2ddb4979c (Bug 1224038) for regressing all the things.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Confirmed fixed with backout
(In reply to Hannes Verschore [:h4writer] from comment #5) > Recorded range: > - > http://hg.mozilla.org/integration/mozilla-inbound/ > pushloghtml?fromchange=a621c6d49da1&tochange=f3d2ddb4979c Since awfy now can work on revision granularity instead of push granularity the regression range reported was empty. Test to see if this new link works better: - http://hg.mozilla.org/integration/mozilla-inbound/log?rev=a621c6d49da1%3A%3Af3d2ddb4979c%20and%20!a621c6d49da1
Okay, so I've got the perf up to snuff, at least locally. The first part is just some new API to make the implementation of the second part easier.
As we discussed on IRC, this pre-caches the lookup UIDs so that we only have to hit the second level table once per lookup. There's still second-level hashing for the key when matching, but balanced against the faster barriers, this seems to break even.
Attachment #8710678 - Flags: review?(sphink)
Attachment #8710676 - Flags: review?(sphink) → review+
Comment on attachment 8710678 [details] [diff] [review] stable_ObjectGroupCompartment_NewEntry-v1.diff Review of attachment 8710678 [details] [diff] [review]: ----------------------------------------------------------------- Very nice!
Attachment #8710678 - Flags: review?(sphink) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/27a5ca4a5e74883481ff4fb1bd330d6af41312c9 Bug 1224038 - Part 1: Add infallible versions of uid and hashcode getters; r=sfink https://hg.mozilla.org/integration/mozilla-inbound/rev/f8c3e1e0e597515ee78c04d561a88669054710d9 Bug 1224038 - Part 2: Use stable hashing for NewTable; r=sfink
https://hg.mozilla.org/integration/mozilla-inbound/rev/9d90b6b2c0ef5ed63706675ab7926d623008ba95 Backout f8c3e1e0e597515ee78c04d561a88669054710d9 (Bug 1224038) for bustage on a CLOSED TREE. https://hg.mozilla.org/integration/mozilla-inbound/rev/f1e2fa50448fc750e8696bbea560e7abbe8d4fb9 Backout 27a5ca4a5e74883481ff4fb1bd330d6af41312c9 (Bug 1224038) for bustage on a CLOSED TREE.
https://treeherder.mozilla.org/logviewer.html#?job_id=20530143&repo=mozilla-inbound RecursionGuard failing. I guess NewTable is used from the parsing thread.
Still crashing on the UID table. I'm not seeing that crash or any TSAN errors in the same place locally. But then I have a 60MiB file filled with TSAN errors from the first 50000 mochitests, so it's quite possible I just am not seeing it.
I don't think it should be required, but let's try it with SequentiallyConsistent. All the articles I could find online for counters use it, so maybe there's a potential ordering I'm not considering?
It's still busted, but I strongly suspect this may be related to bug 1232229, as the assertion here could only possibly happen if we touched a zone from multiple threads at once.
https://hg.mozilla.org/integration/mozilla-inbound/rev/a701a038de28c0e7b2fac1f22135a4afdf5d69b6 Bug 1224038 - Part 1: Add infallible versions of uid and hashcode getters; r=sfink https://hg.mozilla.org/integration/mozilla-inbound/rev/33a1af83a77f29587c9e301aea9d4cb944adb4b5 Bug 1224038 - Part 2: Use stable hashing for NewTable; r=sfink
No sign of that race anymore, so I think my theory in comment 21 is correct.
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea73bd078111d7c6decddfb0242265f9062d5207 Backed out changeset 33a1af83a77f (bug 1224038) for breaking SM tests. https://hg.mozilla.org/integration/mozilla-inbound/rev/df0be98e74d9fc8801a8ff435725c7467b72cfe2 Backed out changset a701a038de28 (bug 1224038) for breaking SM tests.
Failed on various SM(arm) and SM(e) on m-i that did not fail in try. Must be something that landed in the last day or so.
This was a bad interaction with bug 1252329. I missed the fact that the Entry fixupAfterMovingGC pulls out is a copy not a ref, so I was not updating |associated|. Oddly, the test only fails locally in clang and not in gcc, explaining why I missed it locally. In any case, everything else seemed to be green, so I'm going to try relanding.
https://hg.mozilla.org/integration/mozilla-inbound/rev/84e0586b8c81be9dea35c2d192cc6693baed8f7a Bug 1224038 - Part 1: Add infallible versions of uid and hashcode getters; r=sfink https://hg.mozilla.org/integration/mozilla-inbound/rev/b20234ac6cf465c59bbc939560479c223bc282bf Bug 1224038 - Part 2: Use stable hashing for NewTable; r=sfink
You need to log in before you can comment on or make changes to this bug.