Closed Bug 1209754 Opened 4 years ago Closed 4 years ago

Handle off-thread barriers on a case-by-case basis

Categories

(Core :: JavaScript: GC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: terrence, Assigned: terrence)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Currently, our barriers are named like {un}putFooFrom{Any,Main}Thread, where the Any varieties detect off-thread usage and just return early. IIRC, the reason I did this was to handle some ExclusiveContext parsing path where it was hard to disambiguate on and off-thread usage. For reasons I don't recall, waiting for the is-it-cross-generation check to fail slightly later was running into problems.

In any case, it occurred to me that we should probably just do the work to handle this in the off-thread code by checking ecx->isJSContext() or such so that we can get rid of the slow CurrentThreadCanAccessRuntime and of course, the horrible footgunness of it.

Initial shell testing didn't turn up any places where we're currently using this insanity; so off to try with it:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=49cdf7e69f26
Comment on attachment 8667552 [details] [diff] [review]
handle_offthread_barriers_on_a_case_by_case_basis-v0.diff

Try run is looking good.
Attachment #8667552 - Flags: review?(jcoppeard)
Comment on attachment 8667552 [details] [diff] [review]
handle_offthread_barriers_on_a_case_by_case_basis-v0.diff

Review of attachment 8667552 [details] [diff] [review]:
-----------------------------------------------------------------

That's much nicer.

> we should probably just do the work to handle this in the off-thread code by checking ecx->isJSContext() 

I couldn't see any instances of this check in the patch, so does that mean we weren't even using this any more?
Attachment #8667552 - Flags: review?(jcoppeard) → review+
(In reply to Jon Coppeard (:jonco) from comment #2)
> Comment on attachment 8667552 [details] [diff] [review]
> handle_offthread_barriers_on_a_case_by_case_basis-v0.diff
> 
> Review of attachment 8667552 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> That's much nicer.
> 
> > we should probably just do the work to handle this in the off-thread code by checking ecx->isJSContext() 
> 
> I couldn't see any instances of this check in the patch, so does that mean
> we weren't even using this any more?

AFAICT, yes, that is the case.
https://hg.mozilla.org/mozilla-central/rev/f3311acb9940
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.