Open Bug 1583674 Opened 6 years ago Updated 3 years ago

Increase the size of nsChangeHint

Categories

(Core :: Layout, enhancement, P3)

enhancement

Tracking

()

People

(Reporter: boris, Unassigned)

Details

It seems nsChangeHint is full (i.e. all 31 bits are used), so it's impossible to add new hints now. Perhaps it's worth to increase its size?

Well, depends on whether we need to add more, no? Not impossible to do, but note that it effectively grows the size of Element (since we store it in the servo data), so not great.

I assume you want to add a new change hint, but it may be worth looking at the ones we have and whether we can crib from some of them.

But yeah, can be done, though maybe worth trying to not do it.

I see. I should go through the existing hints first. Hope it's no need to add a new one.

Adding new change hints is one of the basic ways we add new optimizations to layout code, so I think we should assume that we're going to need to add more sooner or later, and if have a reason to do it now, we should increase nsChangeHint to 64-bit.

Move to Layout component because this should be a general layout bug.

Component: CSS Parsing and Computation → Layout

(In reply to David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-7 from comment #5)

Adding new change hints is one of the basic ways we add new optimizations to layout code, so I think we should assume that we're going to need to add more sooner or later, and if have a reason to do it now, we should increase nsChangeHint to 64-bit.

Sure, I just think that it can be easily avoided, at least for now. For example I think we could remove nsChangeHint_SyncFrameView:

  • It's never used without NS_STYLE_HINT_VISUAL. Which means that all their users also get Repaint and co.
  • It only gates this call.
  • Given the amount of frames that have views (very tiny: object, subdocument, viewport, list control), and that that call should be cheap anyway if the stuff views depend on doesn't change, seems we should just remove it. That hint doesn't seem to be buying us anything.

Anyhow, if we want to grow nsChangeHint, we can do it without growing per-element memory usage, seems we should have padding here... But that'd be a bit ugly and I'd prefer to avoid that if possible.

David, does the above make sense?

I guess you're not cc'd when you comment. See above :)

Flags: needinfo?(dbaron)

Sure, if you have reasonable ways around it for now, that's fine.

Maybe there's a reasonable amount of engineering work we can do to free up two bits or so without removing optimizations. But I'd prefer not to end up with a culture where we then have to keep doing that to get a third bit, fourth bit, etc., for increasingly large amounts of engineering work, in order to earn new bits back. There's definitely a point where we should increase the size. And how quickly we get to that point affects how valuable it is to save the one or two bits that we can do easily.

Flags: needinfo?(dbaron)

Bug 1585863 and bug 1585858 should give us two change hint bits, for whenever we need them.

Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.