New issue
Advanced search Search tips

Issue 773880 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Reduce memory used by InvalidationSets

Reported by r...@opera.com, Oct 11 2017

Issue description

InvalidationSets can use a lot of memory on sites like facebook which uses a lot of style rules. We might have a lot of invalidation sets created for self invalidation. For instance, the rule will create an invalidation set for each of .a, .b, and #c which are equal:

.a { ... }
.b { ... }
#c { ... }

It's possible to represent these invalidation sets using a singleton DescendantInvalidationSet object.

I have run heap profiling on facebook with and without a patch introducing such a singleton object which the following tracing config:

{
  "startup_duration": 40,
  "result_file": "/tmp/trace.json",
  "trace_config": {
    "included_categories": ["disabled-by-default-memory-infra"],
    "excluded_categories": ["*"],
    "memory_dump_config": {
      "triggers": [
        { "mode": "light", "periodic_interval_ms": 4000 },
        { "mode": "detailed", "periodic_interval_ms": 4000 }
      ]
    }
  }
}

and this command line:

content_shell --no-sandbox --enable-heap-profiling --trace-config-file=./trace.config http://www.facebook.com

The session wasn't exactly the same, this being a live site, but the ratio of memory used for invalidation sets per selector decreased substantially. See attached images.

 
without-patch.png
10.2 KB View Download
with-patch.png
10.1 KB View Download

Comment 1 by r...@opera.com, Oct 11 2017

Status: Started (was: Assigned)
The patch:

https://chromium-review.googlesource.com/#/c/chromium/src/+/713859

Comment 2 by meade@chromium.org, Oct 12 2017

Labels: Update-Quarterly
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f668ed6c79c55fd9641ca76bc1109d10257bac9c

commit f668ed6c79c55fd9641ca76bc1109d10257bac9c
Author: Rune Lillesveen <rune@opera.com>
Date: Fri Oct 13 13:48:40 2017

Use a singleton invalidation set for self invalidations.

We used to create separate invalidation set instances for every single
invalidation set which just contains InvalidatesSelf(). That is, every
simple selector which only appears in the rightmost compound selector:

.a {}
#b:hover {}
#parent > .c {}

The invalidation sets for ".a", "#b", ".c", and ":hover" above are all
the same with only InvalidatesSelf() set.

Instead we can use a singleton invalidation set which is shared for all
such invalidation sets. If we later add more features to the set, we
replace the singleton with a new DescendantInvalidationSet instance
with InvalidatesSelf() set and add new features to that set.

This reduces memory use for invalidation sets from ~1MB to ~256kB on
facebook.com (see measurements in 773880).

Bug:  773880 
Change-Id: I1018cea1f51628f1f940c3722cc2b48a3164a777
Reviewed-on: https://chromium-review.googlesource.com/713859
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Commit-Queue: Rune Lillesveen <rune@opera.com>
Cr-Commit-Position: refs/heads/master@{#508692}
[modify] https://crrev.com/f668ed6c79c55fd9641ca76bc1109d10257bac9c/third_party/WebKit/Source/core/css/RuleFeature.cpp
[modify] https://crrev.com/f668ed6c79c55fd9641ca76bc1109d10257bac9c/third_party/WebKit/Source/core/css/RuleFeature.h
[modify] https://crrev.com/f668ed6c79c55fd9641ca76bc1109d10257bac9c/third_party/WebKit/Source/core/css/RuleFeatureSetTest.cpp
[modify] https://crrev.com/f668ed6c79c55fd9641ca76bc1109d10257bac9c/third_party/WebKit/Source/core/css/SelectRuleFeatureSet.cpp
[modify] https://crrev.com/f668ed6c79c55fd9641ca76bc1109d10257bac9c/third_party/WebKit/Source/core/css/invalidation/InvalidationSet.cpp
[modify] https://crrev.com/f668ed6c79c55fd9641ca76bc1109d10257bac9c/third_party/WebKit/Source/core/css/invalidation/InvalidationSet.h
[modify] https://crrev.com/f668ed6c79c55fd9641ca76bc1109d10257bac9c/third_party/WebKit/Source/core/css/invalidation/InvalidationSetTest.cpp

Comment 4 by r...@opera.com, Oct 16 2017

Further ideas:

* Measure sites using many web components like polymer apps.

* We now add all extracted features in the compound selector to invalidation sets. Instead we could guess which is the most specific one and add that one. For instance, if we have ".a .b.c {}" or ".a #b.c {}", add only one of b/c to the invalidation set for ".a".

Comment 5 by r...@opera.com, Oct 16 2017

Status: Assigned (was: Started)
Project Member

Comment 6 by sheriffbot@chromium.org, Oct 19 2017

Labels: Hotlist-Recharge-BouncingOwner
Owner: ----
Status: Untriaged (was: Assigned)
The assigned owner "rune@opera.com" is not able to receive e-mails, please re-triage.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 7 by nainar@chromium.org, Oct 19 2017

Labels: Hotlist-Reassign-In-Nov
Owner: nainar@chromium.org
Status: Assigned (was: Untriaged)
Labels: -Hotlist-Recharge-BouncingOwner -Hotlist-Reassign-In-Nov
Owner: futhark@chromium.org
Status: Fixed (was: Assigned)
I think I've done the most important improvement here. Even though there are ideas for improvement, I think we can close this for now.

Sign in to add a comment