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

NEW
Assigned to

Status

()

enhancement
P2
normal
2 years ago
2 years ago

People

(Reporter: xidorn, Assigned: xidorn)

Tracking

(Blocks 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 affected)

Details

Assignee

Description

2 years ago
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.
Assignee

Comment 2

2 years ago
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.
Assignee

Comment 3

2 years ago
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?
Assignee

Comment 5

2 years ago
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).
Assignee

Comment 7

2 years ago
(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
Assignee

Comment 8

2 years ago
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.