New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 642722 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature

Blocked on:
issue 666030
issue 685623
issue 692932

Blocking:
issue 319045
issue 660770



Sign in to add a comment

Experiment with lazily parsing CSS properties

Project Member Reported by csharrison@chromium.org, Aug 31 2016

Issue description

See the discussion on loading-dev@:
https://groups.google.com/a/chromium.org/forum/#!topic/loading-dev/pVuMhUZg-qo

The key here is that css rules have very poor utilization rate, so we could speed things up significantly.

Keeping memory use in check + keeping complexity low will be tricky but imo pretty tractable.
 

Comment 1 by suzyh@chromium.org, Aug 31 2016

Labels: Performance Performance-Memory
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 7 2016

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

commit 6e09a67eb013cde927f50ac7931018a15bfdb2ba
Author: kouhei <kouhei@chromium.org>
Date: Fri Oct 07 04:00:23 2016

Cache SubResourceIntegrity checks at Resource

Move SubResourceIntegrity check caches from ScriptResource -> Resource.
This also renames ScriptIntegrityDisposition to generic name ResourceIntegrityDisposition

This is to prepare for CSSStyleSheetResource to use the same logic.
Split from https://codereview.chromium.org/2290983003/.

BUG= 642722 

Review-Url: https://codereview.chromium.org/2398733003
Cr-Commit-Position: refs/heads/master@{#423792}

[modify] https://crrev.com/6e09a67eb013cde927f50ac7931018a15bfdb2ba/third_party/WebKit/Source/core/dom/PendingScript.cpp
[modify] https://crrev.com/6e09a67eb013cde927f50ac7931018a15bfdb2ba/third_party/WebKit/Source/core/fetch/IntegrityMetadata.h
[modify] https://crrev.com/6e09a67eb013cde927f50ac7931018a15bfdb2ba/third_party/WebKit/Source/core/fetch/Resource.cpp
[modify] https://crrev.com/6e09a67eb013cde927f50ac7931018a15bfdb2ba/third_party/WebKit/Source/core/fetch/Resource.h
[modify] https://crrev.com/6e09a67eb013cde927f50ac7931018a15bfdb2ba/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp
[modify] https://crrev.com/6e09a67eb013cde927f50ac7931018a15bfdb2ba/third_party/WebKit/Source/core/fetch/ScriptResource.cpp
[modify] https://crrev.com/6e09a67eb013cde927f50ac7931018a15bfdb2ba/third_party/WebKit/Source/core/fetch/ScriptResource.h

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 7 2016

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

commit 7c28430f36a95d35778beb97b31b1c188f37c76e
Author: nasko <nasko@chromium.org>
Date: Fri Oct 07 20:01:22 2016

Revert of Cache SubResourceIntegrity checks at Resource (patchset #2 id:20001 of https://codereview.chromium.org/2398733003/ )

Reason for revert:
Reverting due to failing PageLoadMetricsBrowserTest.IgnoreDownloads test starting with this CL.

https://build.chromium.org/p/chromium.win/builders/Win7%20%2832%29%20Tests/builds/10984

Original issue's description:
> Cache SubResourceIntegrity checks at Resource
>
> Move SubResourceIntegrity check caches from ScriptResource -> Resource.
> This also renames ScriptIntegrityDisposition to generic name ResourceIntegrityDisposition
>
> This is to prepare for CSSStyleSheetResource to use the same logic.
> Split from https://codereview.chromium.org/2290983003/.
>
> BUG= 642722 
>
> Committed: https://crrev.com/6e09a67eb013cde927f50ac7931018a15bfdb2ba
> Cr-Commit-Position: refs/heads/master@{#423792}

TBR=jww@chromium.org,csharrison@chromium.org,kouhei@chromium.org
BUG= 642722 

Review-Url: https://codereview.chromium.org/2400723005
Cr-Commit-Position: refs/heads/master@{#423946}

[modify] https://crrev.com/7c28430f36a95d35778beb97b31b1c188f37c76e/third_party/WebKit/Source/core/dom/PendingScript.cpp
[modify] https://crrev.com/7c28430f36a95d35778beb97b31b1c188f37c76e/third_party/WebKit/Source/core/fetch/IntegrityMetadata.h
[modify] https://crrev.com/7c28430f36a95d35778beb97b31b1c188f37c76e/third_party/WebKit/Source/core/fetch/Resource.cpp
[modify] https://crrev.com/7c28430f36a95d35778beb97b31b1c188f37c76e/third_party/WebKit/Source/core/fetch/Resource.h
[modify] https://crrev.com/7c28430f36a95d35778beb97b31b1c188f37c76e/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp
[modify] https://crrev.com/7c28430f36a95d35778beb97b31b1c188f37c76e/third_party/WebKit/Source/core/fetch/ScriptResource.cpp
[modify] https://crrev.com/7c28430f36a95d35778beb97b31b1c188f37c76e/third_party/WebKit/Source/core/fetch/ScriptResource.h

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 7 2016

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

commit 7e25c52eaa56c35f3afec7265e5f332c261c75b9
Author: nasko <nasko@chromium.org>
Date: Fri Oct 07 23:19:39 2016

Reland of Cache SubResourceIntegrity checks at Resource (patchset #2 id:20001 of https://codereview… (patchset #1 id:1 of https://codereview.chromium.org/2400723005/ )

Reason for revert:
Reverting the revert, as the test continues failing.

Original issue's description:
> Revert of Cache SubResourceIntegrity checks at Resource (patchset #2 id:20001 of https://codereview.chromium.org/2398733003/ )
>
> Reason for revert:
> Reverting due to failing PageLoadMetricsBrowserTest.IgnoreDownloads test starting with this CL.
>
> https://build.chromium.org/p/chromium.win/builders/Win7%20%2832%29%20Tests/builds/10984
>
> Original issue's description:
> > Cache SubResourceIntegrity checks at Resource
> >
> > Move SubResourceIntegrity check caches from ScriptResource -> Resource.
> > This also renames ScriptIntegrityDisposition to generic name ResourceIntegrityDisposition
> >
> > This is to prepare for CSSStyleSheetResource to use the same logic.
> > Split from https://codereview.chromium.org/2290983003/.
> >
> > BUG= 642722 
> >
> > Committed: https://crrev.com/6e09a67eb013cde927f50ac7931018a15bfdb2ba
> > Cr-Commit-Position: refs/heads/master@{#423792}
>
> TBR=jww@chromium.org,csharrison@chromium.org,kouhei@chromium.org
> BUG= 642722 
>
> Committed: https://crrev.com/7c28430f36a95d35778beb97b31b1c188f37c76e
> Cr-Commit-Position: refs/heads/master@{#423946}

TBR=csharrison@chromium.org,jww@chromium.org,kouhei@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 642722 

Review-Url: https://codereview.chromium.org/2398733005
Cr-Commit-Position: refs/heads/master@{#424007}

[modify] https://crrev.com/7e25c52eaa56c35f3afec7265e5f332c261c75b9/third_party/WebKit/Source/core/dom/PendingScript.cpp
[modify] https://crrev.com/7e25c52eaa56c35f3afec7265e5f332c261c75b9/third_party/WebKit/Source/core/fetch/IntegrityMetadata.h
[modify] https://crrev.com/7e25c52eaa56c35f3afec7265e5f332c261c75b9/third_party/WebKit/Source/core/fetch/Resource.cpp
[modify] https://crrev.com/7e25c52eaa56c35f3afec7265e5f332c261c75b9/third_party/WebKit/Source/core/fetch/Resource.h
[modify] https://crrev.com/7e25c52eaa56c35f3afec7265e5f332c261c75b9/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp
[modify] https://crrev.com/7e25c52eaa56c35f3afec7265e5f332c261c75b9/third_party/WebKit/Source/core/fetch/ScriptResource.cpp
[modify] https://crrev.com/7e25c52eaa56c35f3afec7265e5f332c261c75b9/third_party/WebKit/Source/core/fetch/ScriptResource.h

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 13 2016

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

commit afa48c3ed31f7229c306ce7cda62111be593f719
Author: kouhei <kouhei@chromium.org>
Date: Thu Oct 13 03:04:01 2016

CSSStyleSheetResource should cache decoded text instead of raw bytes

Before this CL, CSSStyleSheetResource kept undecoded text alive, and
decoded text whenever it was requested.

After this CL, the decoded text is kept alive as cache.
It discards undecoded text when the whole sheet text is loaded to avoid
unnecessary memory usage.

As a side-effect, CSSStyleSheetResource::sheetText is no longer const,
as it may cache m_decodedSheetText.

BUG= 642722 ,  653502 

Review-Url: https://codereview.chromium.org/2290983003
Cr-Commit-Position: refs/heads/master@{#424941}

[add] https://crrev.com/afa48c3ed31f7229c306ce7cda62111be593f719/third_party/WebKit/LayoutTests/http/tests/security/subresourceIntegrity/resources/mark-result-red.css
[add] https://crrev.com/afa48c3ed31f7229c306ce7cda62111be593f719/third_party/WebKit/LayoutTests/http/tests/security/subresourceIntegrity/resources/mark-result2-blue.css
[add] https://crrev.com/afa48c3ed31f7229c306ce7cda62111be593f719/third_party/WebKit/LayoutTests/http/tests/security/subresourceIntegrity/subresource-integrity-load-css-after-failed-integrity.html
[modify] https://crrev.com/afa48c3ed31f7229c306ce7cda62111be593f719/third_party/WebKit/Source/core/fetch/CSSStyleSheetResource.cpp
[modify] https://crrev.com/afa48c3ed31f7229c306ce7cda62111be593f719/third_party/WebKit/Source/core/fetch/CSSStyleSheetResource.h
[modify] https://crrev.com/afa48c3ed31f7229c306ce7cda62111be593f719/third_party/WebKit/Source/core/fetch/Resource.cpp
[modify] https://crrev.com/afa48c3ed31f7229c306ce7cda62111be593f719/third_party/WebKit/Source/core/fetch/Resource.h
[modify] https://crrev.com/afa48c3ed31f7229c306ce7cda62111be593f719/third_party/WebKit/Source/core/html/HTMLLinkElement.cpp
[modify] https://crrev.com/afa48c3ed31f7229c306ce7cda62111be593f719/third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp

Comment 7 by meade@chromium.org, Oct 13 2016

Cc: meade@chromium.org
Labels: Objective
Adding Style team tracking tag.
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 26 2016

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

commit a0b2f70553233e7e96d3060d2c8d42c0c6fc0ade
Author: csharrison <csharrison@chromium.org>
Date: Wed Oct 26 13:44:24 2016

This CL adds an experimental runtime enabled feature to defer
parsing style rule properties until they are accessed.

This patch adds two important classes:
CSSLazyParsingState: This class holds all global knowledge required
for parsing. This includes the parsing context, any allocated escaped
strings, and the decoded sheet text.

CSSLazyPropertyParserImpl: this class holds all local information
needed for parsing a single declaration list for a single StyleRule. It
holds a Vector of tokens and a reference to the CSSLazyparsingState.
This is owned by the StyleRule.

See the (slightly stale) design doc at https://goo.gl/ujMdJM

BUG= 642722 

Review-Url: https://codereview.chromium.org/2315923002
Cr-Commit-Position: refs/heads/master@{#427677}

[modify] https://crrev.com/a0b2f70553233e7e96d3060d2c8d42c0c6fc0ade/third_party/WebKit/Source/core/css/BUILD.gn
[modify] https://crrev.com/a0b2f70553233e7e96d3060d2c8d42c0c6fc0ade/third_party/WebKit/Source/core/css/ElementRuleCollector.cpp
[modify] https://crrev.com/a0b2f70553233e7e96d3060d2c8d42c0c6fc0ade/third_party/WebKit/Source/core/css/StylePropertySet.cpp
[modify] https://crrev.com/a0b2f70553233e7e96d3060d2c8d42c0c6fc0ade/third_party/WebKit/Source/core/css/StylePropertySet.h
[modify] https://crrev.com/a0b2f70553233e7e96d3060d2c8d42c0c6fc0ade/third_party/WebKit/Source/core/css/StyleRule.cpp
[modify] https://crrev.com/a0b2f70553233e7e96d3060d2c8d42c0c6fc0ade/third_party/WebKit/Source/core/css/StyleRule.h
[modify] https://crrev.com/a0b2f70553233e7e96d3060d2c8d42c0c6fc0ade/third_party/WebKit/Source/core/css/StyleSheetContents.cpp
[add] https://crrev.com/a0b2f70553233e7e96d3060d2c8d42c0c6fc0ade/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.cpp
[add] https://crrev.com/a0b2f70553233e7e96d3060d2c8d42c0c6fc0ade/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.h
[add] https://crrev.com/a0b2f70553233e7e96d3060d2c8d42c0c6fc0ade/third_party/WebKit/Source/core/css/parser/CSSLazyPropertyParserImpl.cpp
[add] https://crrev.com/a0b2f70553233e7e96d3060d2c8d42c0c6fc0ade/third_party/WebKit/Source/core/css/parser/CSSLazyPropertyParserImpl.h
[modify] https://crrev.com/a0b2f70553233e7e96d3060d2c8d42c0c6fc0ade/third_party/WebKit/Source/core/css/parser/CSSParser.cpp
[modify] https://crrev.com/a0b2f70553233e7e96d3060d2c8d42c0c6fc0ade/third_party/WebKit/Source/core/css/parser/CSSParser.h
[modify] https://crrev.com/a0b2f70553233e7e96d3060d2c8d42c0c6fc0ade/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp
[modify] https://crrev.com/a0b2f70553233e7e96d3060d2c8d42c0c6fc0ade/third_party/WebKit/Source/core/css/parser/CSSParserImpl.h
[modify] https://crrev.com/a0b2f70553233e7e96d3060d2c8d42c0c6fc0ade/third_party/WebKit/Source/core/css/parser/CSSTokenizer.h
[modify] https://crrev.com/a0b2f70553233e7e96d3060d2c8d42c0c6fc0ade/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in

Blocking: 660770
Labels: -Type-Bug Type-Feature
Cc: timloh@chromium.org
Quick update from the first day of the experiment:
We are seeing a 30-35% improvement of parsing author style sheets (at basically all %iles), and update_style is not showing much regression (too much noise). Still, error bars are big.

I expect when [1] lands, update_style will have even less overhead. timloh@ what is the status of the streaming parsing work? I expect that to make the lazy parsing code simpler and more efficient.

[1] https://codereview.chromium.org/2474483002/
I'm actively working on the streaming parser (661854) and hope it'll be done in 2-3 weeks (i.e. after the 56 branch). That might be a bit optimistic though, re-doing the inspector integration might end up harder than I expect.
Sounds great thanks for the update. Let me know if there's any way I can help out. My understanding is that when the streaming parser is finished the lazy work will have a much better win:overhead ratio (no expensive copying of tokens, etc.) 
Blockedon: 666030
Project Member

Comment 17 by bugdroid1@chromium.org, Nov 17 2016

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

commit 37627d6f7cb982de2c40de899bd18b17c6d6943d
Author: csharrison <csharrison@chromium.org>
Date: Thu Nov 17 14:40:32 2016

Add CSSTiming to collect aggregate PLT-level stats about CSS.

We have histograms on micro-metrics like per stylesheet parse time. This
patch adds histograms of how much aggregate time was spent parsing CSS
before the first paint.

This gives us a much better metric for tracking improvements to CSS
related to loading, as improvements directly map to improvements to
first contentful paint.

BUG= 642722 , 666030 

Review-Url: https://codereview.chromium.org/2468913002
Cr-Commit-Position: refs/heads/master@{#432868}

[modify] https://crrev.com/37627d6f7cb982de2c40de899bd18b17c6d6943d/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc
[modify] https://crrev.com/37627d6f7cb982de2c40de899bd18b17c6d6943d/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
[modify] https://crrev.com/37627d6f7cb982de2c40de899bd18b17c6d6943d/chrome/common/page_load_metrics/page_load_metrics_messages.h
[modify] https://crrev.com/37627d6f7cb982de2c40de899bd18b17c6d6943d/chrome/common/page_load_metrics/page_load_timing.cc
[modify] https://crrev.com/37627d6f7cb982de2c40de899bd18b17c6d6943d/chrome/common/page_load_metrics/page_load_timing.h
[modify] https://crrev.com/37627d6f7cb982de2c40de899bd18b17c6d6943d/chrome/renderer/page_load_metrics/metrics_render_frame_observer.cc
[add] https://crrev.com/37627d6f7cb982de2c40de899bd18b17c6d6943d/chrome/test/data/page_load_metrics/page_with_css.html
[add] https://crrev.com/37627d6f7cb982de2c40de899bd18b17c6d6943d/chrome/test/data/page_load_metrics/simple.css
[add] https://crrev.com/37627d6f7cb982de2c40de899bd18b17c6d6943d/chrome/test/data/page_load_metrics/simple.css.mock-http-headers
[modify] https://crrev.com/37627d6f7cb982de2c40de899bd18b17c6d6943d/third_party/WebKit/Source/core/css/BUILD.gn
[add] https://crrev.com/37627d6f7cb982de2c40de899bd18b17c6d6943d/third_party/WebKit/Source/core/css/CSSTiming.cpp
[add] https://crrev.com/37627d6f7cb982de2c40de899bd18b17c6d6943d/third_party/WebKit/Source/core/css/CSSTiming.h
[modify] https://crrev.com/37627d6f7cb982de2c40de899bd18b17c6d6943d/third_party/WebKit/Source/core/css/StyleSheetContents.cpp
[modify] https://crrev.com/37627d6f7cb982de2c40de899bd18b17c6d6943d/third_party/WebKit/Source/core/timing/PerformanceTiming.cpp
[modify] https://crrev.com/37627d6f7cb982de2c40de899bd18b17c6d6943d/third_party/WebKit/Source/core/timing/PerformanceTiming.h
[modify] https://crrev.com/37627d6f7cb982de2c40de899bd18b17c6d6943d/third_party/WebKit/Source/web/WebPerformance.cpp
[modify] https://crrev.com/37627d6f7cb982de2c40de899bd18b17c6d6943d/third_party/WebKit/public/web/WebPerformance.h
[modify] https://crrev.com/37627d6f7cb982de2c40de899bd18b17c6d6943d/tools/metrics/histograms/histograms.xml

Project Member

Comment 19 by bugdroid1@chromium.org, Dec 5 2016

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

commit f8090ce1eb0631947f38a75bd27fa8325facf921
Author: csharrison <csharrison@chromium.org>
Date: Mon Dec 05 22:58:14 2016

[LazyParseCSS] Ensure UseCounting has parity with strict parsing

This patch revamps lazy parsing UseCounting to be as safe and accurate
as possible. For sheets that need counting, we always try to find a
valid Document and use its UseCounter when doing any parsing work. We
maintain a WeakMember to the owning Document to ensure our parsing
context stays valid.

BUG= 642722 

Review-Url: https://codereview.chromium.org/2474483002
Cr-Commit-Position: refs/heads/master@{#436439}

[modify] https://crrev.com/f8090ce1eb0631947f38a75bd27fa8325facf921/third_party/WebKit/Source/core/css/StyleSheetContents.cpp
[modify] https://crrev.com/f8090ce1eb0631947f38a75bd27fa8325facf921/third_party/WebKit/Source/core/css/StyleSheetContents.h
[modify] https://crrev.com/f8090ce1eb0631947f38a75bd27fa8325facf921/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.cpp
[modify] https://crrev.com/f8090ce1eb0631947f38a75bd27fa8325facf921/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.h
[modify] https://crrev.com/f8090ce1eb0631947f38a75bd27fa8325facf921/third_party/WebKit/Source/core/css/parser/CSSLazyParsingTest.cpp

Comment 20 Deleted

Quick update for metrics for rule usage %, from 1 day of canary data:

Rule usage %	PDF	CDF
[0%, 10%]	36	36
(10%, 25%]	43	79
(25%, 50%]	12	91
(50%, 75%]	4	95
(75%, 90%]	1	97
(90%, 100%]	0	97
100%	        3	100

This is very good news, as ~80% of CSS files we're parsing use <= 25% of the rules we lazily defer.

Blockedon: 685623
Components: -Blink>Loader
Labels: Update-Quarterly
Project Member

Comment 24 by bugdroid1@chromium.org, Feb 14 2017

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

commit bd80f2b1e755bce061294a2ecb8a28b988c9a30a
Author: csharrison <csharrison@chromium.org>
Date: Tue Feb 14 16:47:00 2017

[LazyParseCSS] Add field trial configs for testing

BUG= 642722 

Review-Url: https://codereview.chromium.org/2688113003
Cr-Commit-Position: refs/heads/master@{#450381}

[modify] https://crrev.com/bd80f2b1e755bce061294a2ecb8a28b988c9a30a/testing/variations/fieldtrial_testing_config.json

We're a bit concerned with the CSS memory hit we're seeing in the lab due to this change: https://chromium.googlesource.com/chromium/src/+/bd80f2b1e755bce061294a2ecb8a28b988c9a30a

E.g. 5GB regression in renderer process for CNN: https://screenshot.googleplex.com/5wpmVbx9B21
Blockedon: 692932
Do you mean 5MB? This is being tracked in  issue 692932  and I have a WIP CL in flight which is starting to address this. Let me block this bug on 692932 and move discussion to that bug.
Ha! I meant to get some attention to my comment, but not that much attention. Yes, 5MB, sorry about that.
Labels: -Performance
Project Member

Comment 29 by bugdroid1@chromium.org, Aug 21 2017

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

commit 5d173f3f4c19b3564d41853b77da410fdd5d7464
Author: Naina Raisinghani <nainar@google.com>
Date: Mon Aug 21 08:59:09 2017

CSS Parser: Lazy Parsing for Pseudo attributes (before/after)

This patch enables lazy parsing for pseudo attributes ::before and 
this isn't efficient but that is acceptable since lazy parsing is hidden
behind the --experimental-web-platform-features flag. 
 

: :after. As per discussion here: 
https: //groups.google.com/a/chromium.org/forum/#!topic/loading-dev/pVuMhUZg-qo
Bug:  642722 
Change-Id: I708d5b9e7f50fdd77b9443af008904d312de34a9
Reviewed-on: https://chromium-review.googlesource.com/613841
Reviewed-by: Darren Shen <shend@chromium.org>
Commit-Queue: nainar <nainar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495887}
[modify] https://crrev.com/5d173f3f4c19b3564d41853b77da410fdd5d7464/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.cpp
[modify] https://crrev.com/5d173f3f4c19b3564d41853b77da410fdd5d7464/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.h
[modify] https://crrev.com/5d173f3f4c19b3564d41853b77da410fdd5d7464/third_party/WebKit/Source/core/css/parser/CSSLazyParsingTest.cpp
[modify] https://crrev.com/5d173f3f4c19b3564d41853b77da410fdd5d7464/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp

Project Member

Comment 30 by bugdroid1@chromium.org, Aug 29 2017

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

commit d3f5ffac18ff58a1dc98f8e3a8d30f3ffbede693
Author: nainar <nainar@chromium.org>
Date: Tue Aug 29 03:50:43 2017

Revert "CSS Parser: Lazy Parsing for Pseudo attributes (before/after)"

This reverts commit 5d173f3f4c19b3564d41853b77da410fdd5d7464.

Reason for revert: See https://chromium-review.googlesource.com/c/chromium/src/+/613841#message-e939301c56917f4aead8e58f1fa9744b1807f5b0

Original change's description:
> CSS Parser: Lazy Parsing for Pseudo attributes (before/after)
> 
> This patch enables lazy parsing for pseudo attributes ::before and 
> this isn't efficient but that is acceptable since lazy parsing is hidden
> behind the --experimental-web-platform-features flag. 
>  
> 
> : :after. As per discussion here: 
> https: //groups.google.com/a/chromium.org/forum/#!topic/loading-dev/pVuMhUZg-qo
> Bug:  642722 
> Change-Id: I708d5b9e7f50fdd77b9443af008904d312de34a9
> Reviewed-on: https://chromium-review.googlesource.com/613841
> Reviewed-by: Darren Shen <shend@chromium.org>
> Commit-Queue: nainar <nainar@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#495887}

TBR=nainar@chromium.org,shend@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  642722 
Change-Id: I57b9a4ca9e047f2c69e280272184446f7223cc29
Reviewed-on: https://chromium-review.googlesource.com/639770
Reviewed-by: Darren Shen <shend@chromium.org>
Commit-Queue: nainar <nainar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498013}
[modify] https://crrev.com/d3f5ffac18ff58a1dc98f8e3a8d30f3ffbede693/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.cpp
[modify] https://crrev.com/d3f5ffac18ff58a1dc98f8e3a8d30f3ffbede693/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.h
[modify] https://crrev.com/d3f5ffac18ff58a1dc98f8e3a8d30f3ffbede693/third_party/WebKit/Source/core/css/parser/CSSLazyParsingTest.cpp
[modify] https://crrev.com/d3f5ffac18ff58a1dc98f8e3a8d30f3ffbede693/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp

Cc: csharrison@chromium.org
Owner: nainar@chromium.org
Taking over ownership of this bug since I will be actively working on this. 
Sounds good, happy to help with reviews, etc.
Blocking: 319045
Project Member

Comment 34 by bugdroid1@chromium.org, Sep 26 2017

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

commit 100f27a3bb5677b0205106a154f26365c27e785f
Author: Naina Raisinghani <nainar@chromium.org>
Date: Tue Sep 26 11:06:57 2017

CSS Parser: Lazy Parsing for Pseudo attributes (before/after)

Given sample css as follows:
.foo::before {
	display: inline;
	content: attr(bar);
	color: red;
}

We can now parse the content of the .foo::before block.

We maintain whether or not the block to be parsed has ::{before,after}
in its prelude on CSSLazyParsingState

If the block's prelude has ::{before,after} and the properties in the 
block contain CSSPropertyContent, we update invalidation sets for 
that block and mark the CSSGlobalRuleSet as dirty. The document will 
then update the CSSGlobalRuleSet.  

Bug:  642722 
Change-Id: Id6094fbb201b4eb3463ddd34dce32f09c4c1478b
Reviewed-on: https://chromium-review.googlesource.com/662084
Reviewed-by: Rune Lillesveen <rune@opera.com>
Reviewed-by: Darren Shen <shend@chromium.org>
Commit-Queue: nainar <nainar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504333}
[add] https://crrev.com/100f27a3bb5677b0205106a154f26365c27e785f/third_party/WebKit/LayoutTests/fast/css/lazy-parsing-pseudo-attribute.html
[modify] https://crrev.com/100f27a3bb5677b0205106a154f26365c27e785f/third_party/WebKit/Source/core/css/RuleFeature.cpp
[modify] https://crrev.com/100f27a3bb5677b0205106a154f26365c27e785f/third_party/WebKit/Source/core/css/RuleFeature.h
[modify] https://crrev.com/100f27a3bb5677b0205106a154f26365c27e785f/third_party/WebKit/Source/core/css/RuleSet.h
[modify] https://crrev.com/100f27a3bb5677b0205106a154f26365c27e785f/third_party/WebKit/Source/core/css/StyleEngine.h
[modify] https://crrev.com/100f27a3bb5677b0205106a154f26365c27e785f/third_party/WebKit/Source/core/css/StylePropertySet.h
[modify] https://crrev.com/100f27a3bb5677b0205106a154f26365c27e785f/third_party/WebKit/Source/core/css/StyleRule.h
[modify] https://crrev.com/100f27a3bb5677b0205106a154f26365c27e785f/third_party/WebKit/Source/core/css/StyleSheetContents.h
[modify] https://crrev.com/100f27a3bb5677b0205106a154f26365c27e785f/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.cpp
[modify] https://crrev.com/100f27a3bb5677b0205106a154f26365c27e785f/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.h
[modify] https://crrev.com/100f27a3bb5677b0205106a154f26365c27e785f/third_party/WebKit/Source/core/css/parser/CSSLazyParsingTest.cpp
[modify] https://crrev.com/100f27a3bb5677b0205106a154f26365c27e785f/third_party/WebKit/Source/core/css/parser/CSSLazyPropertyParserImpl.cpp
[modify] https://crrev.com/100f27a3bb5677b0205106a154f26365c27e785f/third_party/WebKit/Source/core/css/parser/CSSLazyPropertyParserImpl.h
[modify] https://crrev.com/100f27a3bb5677b0205106a154f26365c27e785f/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp
[modify] https://crrev.com/100f27a3bb5677b0205106a154f26365c27e785f/third_party/WebKit/Source/core/dom/Document.cpp

Project Member

Comment 35 by bugdroid1@chromium.org, Sep 28 2017

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

commit 93e97597e1d66ec5ed8c62f31183e7158cdfc94d
Author: Naina Raisinghani <nainar@chromium.org>
Date: Thu Sep 28 04:10:45 2017

Move has_before_or_after_ from CSSLazyParsingState to CSSLazyPropertyParserImpl

This patches moves the bool maintaining state about the prelude of the
block we are parsing (whether or not the prelude include a
::{before,after}) from CSSLazyParsingState to CSLazyPropertyParserImpl.

This is as we have one CSSLazyParsingState per stylesheet, making this 
the incorrect place to maintain the flag. Before this patch if even one 
block in the stylesheet had ::{before,after} we would lazily parse all 
blocks in the stylesheet if the other conditions in 
CSSLazyPropertyparserImpl::parseProperties was met. 

Since each block gets an  instance of the CSSLazyPropertyParser we 
maintain the information there instead. The drawback to this is 
increased memory use. There is a CSSLazyPropertyParser per declaration 
list, so that adds 1 byte of memory for a stylesheet with N deferred lists. 
However N will usually be small as can be seen in histograms 
tracked under Style.TotalLazyRules

Bug:  642722 
Change-Id: Ief6272dd2ae24a31479d9f0c072c1562632a49c1
Reviewed-on: https://chromium-review.googlesource.com/686196
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Rune Lillesveen <rune@opera.com>
Cr-Commit-Position: refs/heads/master@{#504892}
[modify] https://crrev.com/93e97597e1d66ec5ed8c62f31183e7158cdfc94d/third_party/WebKit/Source/core/css/RuleFeature.cpp
[modify] https://crrev.com/93e97597e1d66ec5ed8c62f31183e7158cdfc94d/third_party/WebKit/Source/core/css/RuleFeature.h
[modify] https://crrev.com/93e97597e1d66ec5ed8c62f31183e7158cdfc94d/third_party/WebKit/Source/core/css/StylePropertySet.h
[modify] https://crrev.com/93e97597e1d66ec5ed8c62f31183e7158cdfc94d/third_party/WebKit/Source/core/css/StyleRule.h
[modify] https://crrev.com/93e97597e1d66ec5ed8c62f31183e7158cdfc94d/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.h
[modify] https://crrev.com/93e97597e1d66ec5ed8c62f31183e7158cdfc94d/third_party/WebKit/Source/core/css/parser/CSSLazyPropertyParserImpl.cpp
[modify] https://crrev.com/93e97597e1d66ec5ed8c62f31183e7158cdfc94d/third_party/WebKit/Source/core/css/parser/CSSLazyPropertyParserImpl.h

Project Member

Comment 36 by bugdroid1@chromium.org, Nov 23 2017

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

commit 5e5f476234f533f9cbe7c4616078bf540a948461
Author: Naina Raisinghani <nainar@chromium.org>
Date: Thu Nov 23 23:56:54 2017

[LazyParseCSS] Add field trial configs for testing

BUG= 642722 

Change-Id: I189dc92049293c27b4caad52fc7b0a11e6242edd
Reviewed-on: https://chromium-review.googlesource.com/784030
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Commit-Queue: nainar <nainar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519030}
[modify] https://crrev.com/5e5f476234f533f9cbe7c4616078bf540a948461/testing/variations/fieldtrial_testing_config.json

Labels: -Update-Quarterly
Owner: csharrison@chromium.org
Switching owners as I am transitioning away from Chrome
Project Member

Comment 39 by bugdroid1@chromium.org, Mar 5 2018

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

commit 85b8a01246c24636576d00b1e1dcaa574c29fc52
Author: Charles Harrison <csharrison@chromium.org>
Date: Mon Mar 05 16:32:10 2018

LazyParseCSS: Enable by default

If everything goes smoothly we can remove the flag in M68.

Bug:  642722 
Change-Id: Ica2eb3037f204605146fc36edc8b0ce91a8c2b3a
Reviewed-on: https://chromium-review.googlesource.com/947775
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540838}
[modify] https://crrev.com/85b8a01246c24636576d00b1e1dcaa574c29fc52/content/public/common/content_features.cc

Status: Fixed (was: Started)
Calling it fixed. Overall this yielded a 1% improvement on TTFCP on Android, with a 30% improvement in overall CSS parsing. We didn't see any significant memory regression.

Sign in to add a comment