This is split off from bug 844769, since we decided to land a simpler solution for that.
Created attachment 8790980 [details] [diff] [review] bug844769-forward-for-rekey-v0.diff This is Jon's rekey patch from bug 844769, r+'d by Steve. We should go ahead and land this as I think it's just an improvement over the semantics that are there now.
Created attachment 8790982 [details] [diff] [review] do_not_fire_read_barriers_omt-wip1.diff This is the rebased main patch from bug 844769. It does not build on top of Jon's fixed rekeying patch. For reasons that I cannot divine, e.rekeyFront(l, Move(entry)) is triggering the [deleted] copy constructor on InitialShapeEntry. Spent too much time banging my head against this today: maybe someone else can figure out what C++ wants this time.
It's because by the time you get to http://dxr.mozilla.org/mozilla-central/source/js/public/HashTable.h#1856 typename HashTableEntry<T>::NonConstT t(mozilla::Move(*p)); it is templatized on «const» T due to http://dxr.mozilla.org/mozilla-central/source/js/public/HashTable.h#326 typedef detail::HashTable<const T, SetOps, AllocPolicy> Impl; inside HashSet. If you remove the const, it works. Or if you explicitly cast: typename Entry::NonConstT temp(const_cast<typename Entry::NonConstT&&>(*p)); which is perhaps the right thing, given all the const casting in HashSet::replaceKey?
Does this fix any detectable race(s), or is it a tidying up that has no effect on the race behaviour?
(In reply to Julian Seward [:jseward] from comment #4) > Does this fix any detectable race(s), or is it a tidying up that has > no effect on the race behaviour? No, Jon's patch in bug 844769 fixed the one race we know about here. This is just so that we can assert unilaterally instead of making a thread-safe branch.
<Waldo> sfink, terrence: I'd probably const_cast to a normal lvalue reference, then Move that
The do_not_fire_read_barriers_omt patch causes assertion failures on try. The read-barriered members of ObjectGroupCompartment::PlainObjectEntry are causing barriers to fire during off-main-thread parsing. More work required.
Created attachment 8803358 [details] [diff] [review] bug1302559-remove-omt-barriers Rebased patch.
Steve, if you've got some sparse cycles, do you think you could take a look at this? I think we just need to change the code relating to ObjectGroupCompartment::PlainObjectEntry in the same way as we did for the other tables that can be access off main thread.
Created attachment 8803941 [details] [diff] [review] bug1302559-remove-omt-barriers v2 Maybe it's a simple as this... https://treeherder.mozilla.org/#/jobs?repo=try&revision=75176ebcf78aeb762ae7cd38fdb04de6f2a0a12a
Nope, lots of things (e.g. TI) end up triggering read barriers. I'm coming to the opinion that this approach is not really an improvement. We have to thread a context through everywhere (and we don't always have one - e.g. IonBuilder) and end up making a dynamic check on that. Currently we have to do two checks when we fire the read barrier (on the zone's needsIncrementalBarrier flag and on the mark bits), so this might make OMT code marginally faster but I doubt it would be significant. And I don't think it really makes us any safer.