Open Bug 1422416 Opened 3 years ago Updated 3 years ago

stylo: No need to mark uncacheable for style adjustment based on reset properties only

Categories

(Core :: CSS Parsing and Computation, enhancement, P2)

enhancement

Tracking

()

Tracking Status
firefox59 --- affected

People

(Reporter: xidorn, Assigned: xidorn)

References

(Blocks 1 open bug)

Details

Currently we mark styles uncacheable for rule cache as soon as style adjuster changes anything. However, this isn't really necessary.

There are cases where adjustment is purely based on some other reset properties and the adjustment itself is idempotent (actually, it is likely a bug if any of the adjustment is not idempotent). In that case, we should be able to allow caching reset properties.

We can put rule condition into StyleAdjuster for this, and invoke set_uncacheable when necessary.
What is not true? I was actually looking at modified_reset check when I filed this bug, because I don't think it is necessary to avoid caching in all those cases.
By "changes anything" in comment 0, I mean, when it changes any reset property. This seems to be too coarse-grained to me.
We either cache all reset structs or none, so it looks to me it's necessary?
If we don't cache all reset structs together, we cannot cache adjustment because there could be inter-dependency between different reset structs. That's the case for Gecko.

But since we cache all reset structs together in Stylo, we should be able to cache post-adjustment result.
(In reply to Xidorn Quan [:xidorn] UTC-6 (less responsive Nov 5 ~ Dec 16) from comment #5)
> If we don't cache all reset structs together, we cannot cache adjustment
> because there could be inter-dependency between different reset structs.
> That's the case for Gecko.
> 
> But since we cache all reset structs together in Stylo, we should be able to
> cache post-adjustment result.

I see what you mean...

Is that the case that there's not inherited -> reset dependence? If it is, that doesn't work in presence of inheritance. And that seems to be the case already...

For example, if we have two elements with different writing mode, they may match the same rules, but the reset structs end up being different (due to adjust_for_writing_mode).
(In reply to Emilio Cobos Álvarez [:emilio] from comment #6)
> Is that the case that there's not inherited -> reset dependence? If it is,
> that doesn't work in presence of inheritance. And that seems to be the case
> already...

There are definitely some adjustments which are uncacheable, and we would need to mark them explicitly. But there are many adjustments which are cacheable. I intend to improve caching for this.

> For example, if we have two elements with different writing mode, they may
> match the same rules, but the reset structs end up being different (due to
> adjust_for_writing_mode).

This would be a case where we need to tell callsite of style adjuster that we have done some adjustment which makes reset structs not cacheable.

adjust_for_writing_mode depends on parent's writing-mode to change display, so naturally it makes reset structs not cacheable.
Assignee: nobody → xidorn+moz
I have a patch for this now, and have pushed to try. We will see whether that would break anything.
Priority: -- → P2
You need to log in before you can comment on or make changes to this bug.