Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: JSManaged vs. JSTraceable objects #32102

Open
sagudev opened this issue Apr 17, 2024 · 8 comments
Open

RFC: JSManaged vs. JSTraceable objects #32102

sagudev opened this issue Apr 17, 2024 · 8 comments
Labels
B-RFC A request for comments on a proposal

Comments

@sagudev
Copy link
Member

sagudev commented Apr 17, 2024

Generally all objects that implements JSTraceable should be js managed, but some types have nop JSTraceable impl (are not js managed).

But for no_trace and for #26488 having more strict definition would help achieve correctness.

Option A: Create JSManaged (marker) trait as subtrait of JSTraceable

trait JSManaged: JSTraceable {}

trait JSTraceable {
    fn trace(&self);
}

#[derive(JSManaged, JSTraceable)]
struct SomeDomObject {
    reflector: Reflector,
}

impl JSTraceable for i32 {
    fn trace(&self) {
        // nop trace
    }
}

Problems:

How to guarantee that all types that are js managed (have non-trivial JSTraceable) are actually marked as such? More Crown Lints!

Option B: "disallow"/remove nop JSTraceable

Disallow nop JSTraceable so all objects that implement traceable are actually js managed. What would enforce such rule? Nothing, but if any type would get nop JSTraceable this could only result false errors triggered by lints.

Problem:

We would need to use a lot more no_trace (~600 errors are generated when removing nop implementations from mozjs).

Option C: Make JSTraceable only for jsmanaged

This option is similar to option B but it goes in opposite direction as no_trace. Using some typesystem trickery we can do something like this:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=9fa68d01e0dc08f3e24997fd3b31d46d, to dispatch in compile-time which tracer would be run (code would be generated in proc-macro of JSTraceable). If JSTraceable is implemented for type then tracer from trait would be called in other case type does not implement JSTraceable so nop traceable would be called.

Good news is that this would remove the need for no_trace and manual nop tracers impl, but we would not get errors when struct field is missing JSTraceable as "fallback" (NotJSManaged) tracer would always be available. This could be solved using new lint in crown (if struct/enum has JSTraceable field in inside it should implement JSTraceable via derive).

Problem:

Composed types that has one jsmanaged and one not jsmanaged value ex: HashMap<i32, JSmanaged>. This type would also need to be JSTraceable, but it would not be due to bounds requiring both K and V to be JSTraceable (i32 is not). The solution is Trace wrapper that would wrap non-jsmanaged (assured via lints) type and implement nop traceable on it (polar opposite of NoTrace that we have now).

@sagudev sagudev added the B-RFC A request for comments on a proposal label Apr 17, 2024
@mrobinson
Copy link
Member

mrobinson commented Apr 17, 2024

Questions:

  • What does it mean for an object to be "JS managed?"
  • Why do some objects implement JSTraceable , but have no-op implementations? Why not make them not implement JSTraceable?
  • What do JSManaged::js_trace() and JSTraceable::trace()` do differently ie what is the difference between a "trace" and a "JS trace?"

@mrobinson
Copy link
Member

mrobinson commented Apr 17, 2024

Are "JS managed" objects a strict superset of objects that implement JSTraceable? If yes, it would probably make sense for JSTaceable to be a supertrait of JSManaged.

@sagudev
Copy link
Member Author

sagudev commented Apr 17, 2024

What does it mean for an object to be "JS managed?"

That it needs to be GCed.

Why do some objects implement JSTraceable , but have no-op implementations?

So we do not have to write no_trace on even more fields.

Why not make them not implement JSTraceable?

That's alternative solution.

What do JSManaged::js_trace() and JSTraceable::trace()` do differently ie what is the difference between a "trace" and a "JS trace?"

that's just for illustration purposes, in reality both would have same signature and same name.

Are "JS managed" objects a strict superset of objects that implement JSTraceable? If yes, it would probably make sense for JSTaceable to be a supertrait of JSManaged.

I completely forgot that existed. Will add it.

@sagudev
Copy link
Member Author

sagudev commented Apr 17, 2024

Another alternative is something like this:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=9fa68d01e0dc08f3e24997fd3b31d46d.

This code could be generated in proc-macro of JSTraceable, so no_trace and nop tracers would no longer be necessary and JSTraceable would only be implemented on js managed (or partially js managed) types, but you would not get errors when struct is missing JSTraceable as "fallback" (NotJSManaged) tracer would always be available. This could be solved using new lint in crown (if struct/enum has JSTraceable field in inside it should implement JSTraceable).

@mrobinson
Copy link
Member

mrobinson commented Apr 17, 2024

What does it mean for an object to be "JS managed?"

That it needs to be GCed.

So do I understand this correctly that a "JS managed" object is one that needs to be garbage collected, but a JSTraceable object is one that has children members that need to be garbage collected?

@sagudev
Copy link
Member Author

sagudev commented Apr 17, 2024

So do I understand this correct that a "JS managed" object is one that needs to be garbage collected, but a JSTraceable object is one that has children that need to be garbage collected?

JS managed is object that needs to be GCed (either fully or only one of it's fields). JSTraceable is trait that traces all JSManaged types to detect usages between them. Because we know rust primitives are not JSManaged they get nop implementation for their tracer.

Dom objects are js managed, but can have fields that are not js managed, those fields implement nop tracers (or have #[no_trace]).

In general each type that has non-trivial tracer implementation is jsmanaged (usually such struct a field of type Heap somewhere (can be nested)).

EDIT: We also have some doc in tree: https://github.com/servo/servo/blob/main/components/script/docs/JS-Servos-only-GC.md

@sagudev sagudev changed the title RFC: Create (marker) trait for JSManaged objects RFC: JSManaged vs. JSTraceable objects Apr 17, 2024
@mrobinson
Copy link
Member

Hrm. So these traits are essentially: NeedsToBeGarbageCollected and HasGarbageCollectedFields?

@sagudev
Copy link
Member Author

sagudev commented Apr 20, 2024

Hrm. So these traits are essentially: NeedsToBeGarbageCollected and HasGarbageCollectedFields?

Well JSManaged types are for both, while JSTraceable are just types that implements JSTraceable even if they are not GCed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-RFC A request for comments on a proposal
Projects
None yet
Development

No branches or pull requests

2 participants