stylo: Consider making DynamicAtom public so we can devirtualize AddRefing/Relasing them.

RESOLVED DUPLICATE of bug 1362338

Status

()

Core
CSS Parsing and Computation
RESOLVED DUPLICATE of bug 1362338
10 months ago
10 months ago

People

(Reporter: emilio, Unassigned)

Tracking

(Blocks: 2 bugs)

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

10 months ago
Right now this is hot on stylist rebuild times because the Rust's hashmap entry API forces an unconditional clone of the key.

Most of the atoms in stylists are dynamic. I wonder if we could make it public so  addrefing/releasing an atom is a function call, not an FFI call that calls a virtual method.
(Reporter)

Comment 1

10 months ago
Nathan, does it sound reasonable to make DynamicAtom public? We could also entirely devirtualize nsIAtom::AddRef/Release, I guess, though that's harder because it inherits from nsISupports. But marking it |final| the compiler should be able to figure it out, I guess.

The basic idea would be something like what stylo does already:

nsrefcnt
nsIAtom::AddRef()
{
  if (mIsStatic) {
    return 2;
  }
  return static_cast<DynamicAtom*>(this)->AddRef();
}

Where DynamicAtom::AddRef is an inline function. Similarly for Release(). This would avoid us exposing DynamicAtom around.
Flags: needinfo?(nfroyd)
Status: NEW → RESOLVED
Last Resolved: 10 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1362338

Comment 3

10 months ago
Xidorn already dup'd this, but I am confused about "something like what stylo does already"...does stylo glue code try to devirtualize things on its own?
Flags: needinfo?(nfroyd)
(Reporter)

Comment 4

10 months ago
(In reply to Nathan Froyd [:froydnj] from comment #3)
> Xidorn already dup'd this, but I am confused about "something like what
> stylo does already"...does stylo glue code try to devirtualize things on its
> own?

To some extent. We already avoid going through the FFI boundary if mIsStatic is non-zero.
You need to log in before you can comment on or make changes to this bug.