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

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Terrence Cole [:terrence]]

Attachment: handle_offthread_barriers_on_a_case_by_case_basis-v0.diff

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 1

[Terrence Cole [:terrence]]

Comment on attachment 8667552 [details] [diff] [review]
handle_offthread_barriers_on_a_case_by_case_basis-v0.diff

Try run is looking good.

Comment 2

[Jon Coppeard (:jonco)]

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?

Comment 3

[Terrence Cole [:terrence]]

(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.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f3311acb9940

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f3311acb9940