Make CSSParser as fast as the fast path and remove the fast path |
|||||||||||||||||||
Issue descriptionAnimometer misses the CSSParserFastPaths because it uses rotate in some benchmarks, ex. transform = "translate(1px, 1px) rotate(1deg)" If it used the fast path it cuts the script run time considerably: ex. https://codereview.chromium.org/1904633002 where I added support for rotate which removed 40% of the script time. The fast path is also bad because we do a bunch of work before bailing out of it, so non-fast path transforms take a huge perf hit by doing work in the fast path only to end up doing redundant work in the slow path. We should just make the CSSParser and CSSTokenizer much faster and remove all the fast paths. Attached is a benchmark that shows doing transform = "translate(1px, 1px )" <-- note the space before the ) is nearly 3x slower than doing transform = "translate(1px, 1px)" <-- note the missing space because we miss the fast path. The slowness there is both doing redundant work in the fast path, and also that the main parser itself is slower.
,
Apr 22 2016
Things I've found from staring at profiles for an hour of CSSParserImpl::parseValue:
- Time spent copying the CSSParserContext.
- Lots of time inside malloc (~5%)
- Lots of time creating strings (~5%) and hashing them (~4%), and trying to lower() then (~5%), ex. for CSSPrimitiveValue::fromName
- CSSTokenizerInputStream::peek is not inline, but we call it tons (4.5%). pushBack, reconsume and all these tiny functions should also be inline.
Places I think we could start:
- CSSParserImpl's m_context should be:
const CSSParserContext& m_context;
instead of copying the context, there's no reason to copy it. This uses time, ex. KURL/~KURL show up in the profile since we copy the context thousands of times.
- CSSTokenizer::Scope::Scope should use inline capacity for
Vector<CSSParserToken> m_tokens;
Vector<CSSParserTokenType> m_blockStack;
- CSSPrimitiveValue::fromName should not take a String, it should take either a (UChar*, length) or an (LChar*, length), this skips the String copy in CSSParserToken::convertToDimensionWithUnit.
CSSParserToken::convertToDimensionWithUnit should do something like:
if (unit.is8Bit())
m_unit = static_cast<unsigned>(CSSPrimitiveValue::fromName(unit.characters8(), unit.length()));
else
m_unit = static_cast<unsigned>(CSSPrimitiveValue::fromName(unit.characters16(), unit.length()));
then ::fromName should be rewritten to use a trie like:
template<typename CharacterType>
CSSPrimitiveValue::UnitType toUnitType(CharacterType* chars, unsigned length)
{
switch (length) {
case 0:
return CSSPrimitiveValue::UnitType::UserUnits;
case 1:
if (toASCIILower(chars[0]) == 's')
return CSSPrimitiveValue::UnitType::Seconds;
break;
case 2:
switch (toASCIILower(chars[0])) {
case 'c':
switch (toASCIILower(chars[1])) {
case 'h':
return CSSPrimitiveValue::UnitType::Chs;
case 'm':
return CSSPrimitiveValue::UnitType::Centimeters;
}
break;
...
we can auto generate that if needed just like we do for HTML tag names:
https://code.google.com/p/chromium/codesearch#chromium/src/out/Debug/gen/blink/core/HTMLElementLookupTrie.cpp
- We should do the same thing for the CSSValueKeywords, they allocate and discard temporary strings to find the function name.
CSSValueID CSSParserToken::functionId() const
{
if (m_type != FunctionToken)
return CSSValueInvalid;
if (m_id < 0)
m_id = cssValueKeywordID(value()); <-- Secret string allocation!!
return static_cast<CSSValueID>(m_id);
}
We should use a trie for cssValueKeywordID as well, and should not allocate a temporary string, which we then copy into a buffer, and then hash, and then lookup in the keyword table. :(
- Long term we should remove the conversion operators from CSSParserString, and always attempt to just use the bytes directly instead.
ie.
operator String() const
operator AtomicString() const
- Audit all the tiny helper functions and make sure they're inline, ex. peek() should almost certainly be inline, so should reconsume and pushBack.
,
Apr 22 2016
I commented on https://codereview.chromium.org/1319453002/ a while ago noting that we do string to id mappings a ton of different ways and we need to unify them. Maybe now is the time :) CSSParserToken::id() and functionId() don't actually do string allocations since they call into cssValueKeywordID(const CSSParserString& string). The local buffer is for lower-casing since gperf is only mapping the lower-cased versions. If we wanted to avoid the extra copy we'd probably need to ditch gperf. We really shouldn't need to allocate and lower() to call CSSPrimitiveValue::fromName(). I was thinking that parsing longer strings (keywords, property names) we could do multiple comparisons at once, (e.g. along the lines of (*(int64_t*)x | 0x2020202020202020LL) == *(int64_t*)"translat" && (x[8]|0x20) == 'e'), but ARM doesn't support unaligned accesses.
,
Apr 22 2016
Yeah I see it in: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp&sq=package:chromium&l=269&rcl=1461323877 I think we should consider just using a trie for all of these things. If that's too much RO space we could go gperf I guess. We definitely need to fix ::fromName and remove the unitTable() though.
,
Apr 22 2016
Here's the patch where I've been experimenting: https://codereview.chromium.org/1920583002 Hopefully you can take over productionizing the optimizations. There's definitely a lot to be won here. Getting up to the same speed as the fast path is very hard, I suspect we'll need to change the way the tokenizer/parser works at some level, but doing the trie, inline capacity, some inlining, some copy avoidance, etc. results in pretty decent wins.
,
Apr 24 2016
,
May 11 2016
What's the progress here? It should be easy to land the unit parsing and inline capacity patches to get a pretty big win.
,
May 12 2016
I have https://codereview.chromium.org/1938343002 out for review, the other is in progress.
,
May 12 2016
,
May 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1f192ea58ce279fd382efd3a362f8afff9f79cc1 commit 1f192ea58ce279fd382efd3a362f8afff9f79cc1 Author: timloh <timloh@chromium.org> Date: Wed May 25 07:21:40 2016 Make CSSParserImpl::m_context a const reference to avoid copies BUG=605792 Review-Url: https://codereview.chromium.org/2005203002 Cr-Commit-Position: refs/heads/master@{#395821} [modify] https://crrev.com/1f192ea58ce279fd382efd3a362f8afff9f79cc1/third_party/WebKit/Source/core/css/parser/CSSParserImpl.h
,
May 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a753d727f2d92f67b594312a6710c931b37a49b3 commit a753d727f2d92f67b594312a6710c931b37a49b3 Author: timloh <timloh@chromium.org> Date: Wed May 25 10:02:34 2016 Avoid string allocation when parsing CSSPrimitiveValue::UnitType Currently the CSSPrimitiveValue::fromName function, which parses strings to unit types, takes a String argument. This means that the CSS parser needs to allocate a String (since it would only otherwise have a CSSParserString). This patch avoids this allocation using a generated switch-statement trie instead. Generated file: http://paste.ubuntu.com/16652358/ This improves performance of the inline-transform-benchmark test from https://bugs.chromium.org/p/chromium/issues/detail?id=605792, reducing the time spent in CSSParserImpl::parseValue by 10%. Patch originally written by meade@chromium.org: https://codereview.chromium.org/1938343002 BUG=605792,606695 Review-Url: https://codereview.chromium.org/2002383002 Cr-Commit-Position: refs/heads/master@{#395849} [add] https://crrev.com/a753d727f2d92f67b594312a6710c931b37a49b3/third_party/WebKit/Source/build/scripts/make_css_primitive_value_unit_trie.py [add] https://crrev.com/a753d727f2d92f67b594312a6710c931b37a49b3/third_party/WebKit/Source/build/scripts/templates/CSSPrimitiveValueUnitTrie.cpp.tmpl [modify] https://crrev.com/a753d727f2d92f67b594312a6710c931b37a49b3/third_party/WebKit/Source/core/BUILD.gn [modify] https://crrev.com/a753d727f2d92f67b594312a6710c931b37a49b3/third_party/WebKit/Source/core/CoreInitializer.cpp [modify] https://crrev.com/a753d727f2d92f67b594312a6710c931b37a49b3/third_party/WebKit/Source/core/core.gypi [modify] https://crrev.com/a753d727f2d92f67b594312a6710c931b37a49b3/third_party/WebKit/Source/core/core_generated.gyp [modify] https://crrev.com/a753d727f2d92f67b594312a6710c931b37a49b3/third_party/WebKit/Source/core/core_generated.gypi [modify] https://crrev.com/a753d727f2d92f67b594312a6710c931b37a49b3/third_party/WebKit/Source/core/css/CSSPrimitiveValue.cpp [add] https://crrev.com/a753d727f2d92f67b594312a6710c931b37a49b3/third_party/WebKit/Source/core/css/CSSPrimitiveValueUnitTrie.h [add] https://crrev.com/a753d727f2d92f67b594312a6710c931b37a49b3/third_party/WebKit/Source/core/css/CSSPrimitiveValueUnits.in [modify] https://crrev.com/a753d727f2d92f67b594312a6710c931b37a49b3/third_party/WebKit/Source/core/css/cssom/CSSAngleValue.cpp [modify] https://crrev.com/a753d727f2d92f67b594312a6710c931b37a49b3/third_party/WebKit/Source/core/css/cssom/CSSLengthValue.cpp [modify] https://crrev.com/a753d727f2d92f67b594312a6710c931b37a49b3/third_party/WebKit/Source/core/css/parser/CSSParserToken.cpp
,
May 26 2016
,
Jun 1 2016
Moving this nonessential bug to the next milestone. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 3 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e22ff8c96ff8b3fa85e1f51b98daf27d5e68a09d commit e22ff8c96ff8b3fa85e1f51b98daf27d5e68a09d Author: meade <meade@chromium.org> Date: Fri Jun 03 12:01:40 2016 Delete remaining unused function definitionss from CSSPrimitiveValue header These functions were removed from the cpp file in https://codereview.chromium.org/2002383002. BUG=605792,606695 Review-Url: https://codereview.chromium.org/2036923002 Cr-Commit-Position: refs/heads/master@{#397686} [modify] https://crrev.com/e22ff8c96ff8b3fa85e1f51b98daf27d5e68a09d/third_party/WebKit/Source/core/css/CSSPrimitiveValue.h
,
Jun 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0d4987c045a9cbc5033295f0aee38a2456f039bf commit 0d4987c045a9cbc5033295f0aee38a2456f039bf Author: esprehn <esprehn@chromium.org> Date: Mon Jun 27 02:49:45 2016 Avoid copying strings when parsing CSS colors on the slow path. parseHexColor was always calling value().toString() forcing a string copy of every hex color inside the parser. We can instead change Color::parseHexColor to take a StringView and pass value() directly avoiding this string copy. CSSTokenizer::consumeName() also has a fast path which avoids making string copies when the token value doesn't contain escapes. This path didn't work when the input to the parser was a value that's not followed by a semi-colon (ex. color = "#ccc" in JS) because peekWithoutReplacement would return NUL when we accessed the first out of range position, which then aborted the loop, and made us go down the string copy path usually used for escapes. This patch makes parsing colors on the slow path 2x faster: ex. for (var i = 0; i < 50000; ++i) { span.style.color = " #ddd"; // note the space before the #. The color parsing fast path still seems about 2x faster than this optimized path, but this patch gets us much closer to not needing the color parsing fast path and makes whitespace in your colors less terrible for performance. The patch will also help CSS parsing in general by avoiding string copies for every hex color. BUG=605792 Review-Url: https://codereview.chromium.org/2100573002 Cr-Commit-Position: refs/heads/master@{#402097} [modify] https://crrev.com/0d4987c045a9cbc5033295f0aee38a2456f039bf/third_party/WebKit/Source/core/css/parser/CSSPropertyParserHelpers.cpp [modify] https://crrev.com/0d4987c045a9cbc5033295f0aee38a2456f039bf/third_party/WebKit/Source/core/css/parser/CSSTokenizer.cpp [modify] https://crrev.com/0d4987c045a9cbc5033295f0aee38a2456f039bf/third_party/WebKit/Source/core/css/parser/CSSTokenizerInputStream.h [modify] https://crrev.com/0d4987c045a9cbc5033295f0aee38a2456f039bf/third_party/WebKit/Source/platform/graphics/Color.cpp [modify] https://crrev.com/0d4987c045a9cbc5033295f0aee38a2456f039bf/third_party/WebKit/Source/platform/graphics/Color.h
,
Jun 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d04741aea9777fd9b26c98508c262f9c404584de commit d04741aea9777fd9b26c98508c262f9c404584de Author: esprehn <esprehn@chromium.org> Date: Mon Jun 27 05:04:37 2016 Optimize whitespace skipping in the CSSParser. The code was using nextInputChar() which would make a function call, check the length, call StringImpl operator[] which would check is8Bit() then branch looking for a NUL to do replacement, then always call the UChar version of isHTMLSpace, then call consume() which would again call nextInputChar() doing a check on length, String operator[], another branch only to discard the return value entirely, and finally call advance() to move the offset forward. We can do much better than this by introducing CSSTokenizerInputStream::advanceUntilNonWhitespace which doesn't do replacement (since it doesn't matter for whitespace), doesn't do repeated length checks, and specializes on the character type so we can use isHTMLSpace<LChar> when possible. This yields a 15% performance improvement when parsing long runs of whitespace like you might find in a stylesheet when tested using inline style with this simple benchmark: // 32 spaces before the value. for (var i = 0; i < 99999; ++i) span.style.color = " red"; This will benefit any site that has a bunch of whitespace in the stylesheets. Many web components applications (ex. Polymer Shop) have lots of it. BUG=605792 Review-Url: https://codereview.chromium.org/2099683003 Cr-Commit-Position: refs/heads/master@{#402116} [modify] https://crrev.com/d04741aea9777fd9b26c98508c262f9c404584de/third_party/WebKit/Source/core/css/parser/CSSTokenizer.cpp [modify] https://crrev.com/d04741aea9777fd9b26c98508c262f9c404584de/third_party/WebKit/Source/core/css/parser/CSSTokenizer.h [modify] https://crrev.com/d04741aea9777fd9b26c98508c262f9c404584de/third_party/WebKit/Source/core/css/parser/CSSTokenizerInputStream.cpp [modify] https://crrev.com/d04741aea9777fd9b26c98508c262f9c404584de/third_party/WebKit/Source/core/css/parser/CSSTokenizerInputStream.h
,
Jun 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1c652bdeb846f55452909d223837f55f559bdb27 commit 1c652bdeb846f55452909d223837f55f559bdb27 Author: esprehn <esprehn@chromium.org> Date: Mon Jun 27 21:44:32 2016 Add missing const on CSSTokenizerInputStream methods. BUG=605792 Review-Url: https://codereview.chromium.org/2107503002 Cr-Commit-Position: refs/heads/master@{#402300} [modify] https://crrev.com/1c652bdeb846f55452909d223837f55f559bdb27/third_party/WebKit/Source/core/css/parser/CSSTokenizerInputStream.cpp [modify] https://crrev.com/1c652bdeb846f55452909d223837f55f559bdb27/third_party/WebKit/Source/core/css/parser/CSSTokenizerInputStream.h
,
Jun 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c0d8da0516d76101a420989d2fc9ce9444c21c32 commit c0d8da0516d76101a420989d2fc9ce9444c21c32 Author: esprehn <esprehn@chromium.org> Date: Mon Jun 27 23:22:54 2016 Don't use consume() in CSSTokenizer when ignoring the return value. consume() (the no argument version) does a bunch of work handling out of bounds access, NUL replacement characters, and returning the char. Many of the callers just want to increment the offset. We could do that by replacing the call sites with consume(1) which is the "fast" version that doesn't have a return value, but it'd make the code more clear to just inline advance() in the right spots. BUG=605792 Review-Url: https://codereview.chromium.org/2099413002 Cr-Commit-Position: refs/heads/master@{#402328} [modify] https://crrev.com/c0d8da0516d76101a420989d2fc9ce9444c21c32/third_party/WebKit/Source/core/css/parser/CSSTokenizer.cpp [modify] https://crrev.com/c0d8da0516d76101a420989d2fc9ce9444c21c32/third_party/WebKit/Source/core/css/parser/CSSTokenizer.h
,
Jun 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/53ab0155b7a113df1fcbeb1050df6fea27581685 commit 53ab0155b7a113df1fcbeb1050df6fea27581685 Author: esprehn <esprehn@chromium.org> Date: Tue Jun 28 03:53:38 2016 Add inline capacity to CSSTokenizer::m_blockStack. The malloc on the m_blockStack shows up in profiles in Animometer when parsing transforms since we push and pop from this stack for every paren and transforms use function syntax like rotate(10deg). We can instead use inline capacity to skip the malloc when parsing many inline transforms, variables, or other things that use parens. I chose 8 since it's unlikely that you'd nest more than 8 parens, brackets or braces in CSS. We can easily make this number bigger someday too since CSSParserTokenType is just an enum so the storage is very small. BUG=605792 Review-Url: https://codereview.chromium.org/2105463003 Cr-Commit-Position: refs/heads/master@{#402401} [modify] https://crrev.com/53ab0155b7a113df1fcbeb1050df6fea27581685/third_party/WebKit/Source/core/css/parser/CSSTokenizer.cpp [modify] https://crrev.com/53ab0155b7a113df1fcbeb1050df6fea27581685/third_party/WebKit/Source/core/css/parser/CSSTokenizer.h
,
Jul 12 2016
This issue is Pri-1 but has already been moved once. Lowering the priority and moving to the next milestone. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 19 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9d4b30269e660985f657126b21e7ace229ec2e07 commit 9d4b30269e660985f657126b21e7ace229ec2e07 Author: esprehn <esprehn@chromium.org> Date: Tue Jul 19 20:48:12 2016 Add StringView constructor that takes a ref to save a branch in CSSTokenizerInputStream::rangeAt. CSSTokenizerInputStream never has a null m_string so the null check in StringView's constructor is just waste. It also makes the code bigger if we were to inline the function since we need both the null (clear()) and the non-null (set()) code from StringView. Lets introduce a StringView constructor set that takes a StringImpl& and then use it in CSSTokenizerInputStream::rangeAt() allowing us to skip the branch. I then also made the function inline. BUG= 615174 ,605792 Review-Url: https://codereview.chromium.org/2162863002 Cr-Commit-Position: refs/heads/master@{#406375} [modify] https://crrev.com/9d4b30269e660985f657126b21e7ace229ec2e07/third_party/WebKit/Source/core/css/parser/CSSTokenizerInputStream.cpp [modify] https://crrev.com/9d4b30269e660985f657126b21e7ace229ec2e07/third_party/WebKit/Source/core/css/parser/CSSTokenizerInputStream.h [modify] https://crrev.com/9d4b30269e660985f657126b21e7ace229ec2e07/third_party/WebKit/Source/wtf/text/StringView.h [modify] https://crrev.com/9d4b30269e660985f657126b21e7ace229ec2e07/third_party/WebKit/Source/wtf/text/StringViewTest.cpp
,
Jul 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a11575f6ecbe2f649a64f6727382f80a9c4c9880 commit a11575f6ecbe2f649a64f6727382f80a9c4c9880 Author: esprehn <esprehn@chromium.org> Date: Wed Jul 20 02:20:16 2016 Use CSSTokenizerInputStream::peekWithoutReplacement instead of peek(). nextInputChar() is the only caller of peek() that wants NUL replacement, lets have all other callers use peekWithoutReplacement and skip a branch instead. This leaves only nextInputChar() calling peek(0) which lets us merge peek into nextInputChar and since the lookaheadOffset is always 0 we can also simplify the code. BUG=605792 Review-Url: https://codereview.chromium.org/2160943002 Cr-Commit-Position: refs/heads/master@{#406465} [modify] https://crrev.com/a11575f6ecbe2f649a64f6727382f80a9c4c9880/third_party/WebKit/Source/core/css/parser/CSSTokenizer.cpp [modify] https://crrev.com/a11575f6ecbe2f649a64f6727382f80a9c4c9880/third_party/WebKit/Source/core/css/parser/CSSTokenizerInputStream.cpp [modify] https://crrev.com/a11575f6ecbe2f649a64f6727382f80a9c4c9880/third_party/WebKit/Source/core/css/parser/CSSTokenizerInputStream.h
,
Jul 20 2016
,
Jul 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a7b9e5829dda06696a44ad0447219676babc7ef6 commit a7b9e5829dda06696a44ad0447219676babc7ef6 Author: esprehn <esprehn@chromium.org> Date: Fri Jul 22 03:47:30 2016 Replace most callers of nextInputChar with peekWithoutReplacement in CSSTokenizer. This saves us a branch at each call site since we don't need to check for NUL characters. BUG=605792 Review-Url: https://codereview.chromium.org/2167073002 Cr-Commit-Position: refs/heads/master@{#407044} [modify] https://crrev.com/a7b9e5829dda06696a44ad0447219676babc7ef6/third_party/WebKit/Source/core/css/parser/CSSTokenizer.cpp
,
Aug 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/37f87f519d4f1e6d43286e1e433b81e217d3b467 commit 37f87f519d4f1e6d43286e1e433b81e217d3b467 Author: esprehn <esprehn@chromium.org> Date: Tue Aug 09 05:36:03 2016 Merge all of the consumeNumber() logic and make a single call to getDouble. We don't need to do all of the math manually to convert the string into a number value, we can just validate the input and then call getDouble. This is still slower than the fast path which optimistically attempts to parse the whole input as a double followed by a length, but it makes the code simpler which is nice. It also means we now call charactersToDouble the same number of times in the fast and slow paths. I also converted CSSTokenizerInputStream::skipWhilePredicate to branch once on is8Bit so we're not checking every time inside the loop. BUG=605792 Review-Url: https://codereview.chromium.org/2217853002 Cr-Commit-Position: refs/heads/master@{#410588} [modify] https://crrev.com/37f87f519d4f1e6d43286e1e433b81e217d3b467/third_party/WebKit/Source/core/css/parser/CSSTokenizer.cpp [modify] https://crrev.com/37f87f519d4f1e6d43286e1e433b81e217d3b467/third_party/WebKit/Source/core/css/parser/CSSTokenizerInputStream.h
,
Aug 31 2016
,
Aug 31 2016
,
Feb 13 2017
,
Mar 8 2017
,
Aug 12 2017
Triaging for speed, but also there's something about code health/refactoring in this.
,
Sep 26 2017
I might take a look at this since it's related to streaming parser.
,
Sep 28 2017
To get a sense of where we're at, I ran the animometer benchmark from motionmark with a profiler. The most time consuming functions called by CSSParser::ParseValue are:
4409 8.67% 8.67% 7090 13.95% WTF::double_conversion::Strtod
3658 7.20% 15.87% 14437 28.40% blink::CSSTokenizer::ConsumeNumber
3107 6.11% 21.98% 3129 6.16% WTF::double_conversion::Vector::operator[]
2298 4.52% 26.50% 23081 45.40% blink::CSSTokenizer::TokenizeToEOF
1858 3.66% 30.16% 2963 5.83% blink::CSSPrimitiveValue::Create
1727 3.40% 33.55% 10153 19.97% WTF::double_conversion::StringToDoubleConverter::StringToDouble
1659 3.26% 36.82% 20520 40.37% blink::CSSTokenizer::NextToken
1609 3.17% 39.98% 44784 88.10% blink::CSSParserImpl::ParseValue
1298 2.55% 42.54% 1311 2.58% blink::CSSValue::operator==
Observations:
- Seems like parsing doubles is slow, so maybe we should avoid doing that as much as possible.
- Creating CSSPrimitiveValues is also slow due to the multitude of branches.
,
Oct 16 2017
Not actively working on this, marking as Available.
,
Oct 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1fb83262b5f21cb51016fd1bf0802899db982133 commit 1fb83262b5f21cb51016fd1bf0802899db982133 Author: Darren Shen <shend@chromium.org> Date: Wed Oct 25 01:05:12 2017 [CSSParser] Reject properties that cannot use the fast path. When parsing CSS property values, we first do a quick scan to see if the value is simple enough to avoid tokenizing. This works well for things like lengths, where the values have predictable structure. If the scan failed, we do tokenization and parsing as fallback. Hence, for properties that don't use the fast path, we do some extra work. This patch adds an early exit for the fast path code for properties that we know will almost never use the fast path. Bug: 605792 Change-Id: Iba211c844d5faa2c7272d10fd02b127d5c6abe1b Reviewed-on: https://chromium-review.googlesource.com/701957 Reviewed-by: nainar <nainar@chromium.org> Commit-Queue: Darren Shen <shend@chromium.org> Cr-Commit-Position: refs/heads/master@{#511324} [modify] https://crrev.com/1fb83262b5f21cb51016fd1bf0802899db982133/third_party/WebKit/Source/core/css/parser/CSSParserFastPaths.cpp
,
Oct 30 2017
,
Oct 31 2017
Marking as HardBug because it might require a lot of profiling and code changes.
,
Oct 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/10d25b18c1fd645b2ab592971866b2f9e4b2a5e2 commit 10d25b18c1fd645b2ab592971866b2f9e4b2a5e2 Author: Darren Shen <shend@chromium.org> Date: Tue Oct 31 07:37:15 2017 Revert "[CSSParser] Reject properties that cannot use the fast path." This reverts commit 1fb83262b5f21cb51016fd1bf0802899db982133. Reason for revert: Doesn't seem to improve performance. Original change's description: > [CSSParser] Reject properties that cannot use the fast path. > > When parsing CSS property values, we first do a quick scan to see if the > value is simple enough to avoid tokenizing. This works well for things > like lengths, where the values have predictable structure. If the scan > failed, we do tokenization and parsing as fallback. Hence, for properties > that don't use the fast path, we do some extra work. > > This patch adds an early exit for the fast path code for properties that > we know will almost never use the fast path. > > Bug: 605792 > Change-Id: Iba211c844d5faa2c7272d10fd02b127d5c6abe1b > Reviewed-on: https://chromium-review.googlesource.com/701957 > Reviewed-by: nainar <nainar@chromium.org> > Commit-Queue: Darren Shen <shend@chromium.org> > Cr-Commit-Position: refs/heads/master@{#511324} TBR=nainar@chromium.org,shend@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 605792 Change-Id: If73e39e891c892aef1b745b95f45df8ba242af8c Reviewed-on: https://chromium-review.googlesource.com/746421 Reviewed-by: Darren Shen <shend@chromium.org> Commit-Queue: Darren Shen <shend@chromium.org> Cr-Commit-Position: refs/heads/master@{#512772} [modify] https://crrev.com/10d25b18c1fd645b2ab592971866b2f9e4b2a5e2/third_party/WebKit/Source/core/css/parser/CSSParserFastPaths.cpp
,
Dec 6 2017
,
May 8 2018
|
|||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||
Comment 1 by ojan@chromium.org
, Apr 22 2016