Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(51)

Issue 18147020: Remove broken sibling selector optimization (Closed)

Created:
7 years, 5 months ago by leviw_travelin_and_unemployed
Modified:
7 years, 5 months ago
Reviewers:
ojan, eseidel
CC:
blink-reviews, apavlov+blink_chromium.org, dglazkov+blink, eae+blinkwatch, adamk+blink_chromium.org, darktears
Visibility:
Public.

Description

Remove broken sibling selector optimization Blink r154053 landed with a broken optimization for nth-child that this patch removes. Background: nth-child was formerly O(1) and nth-last-child was O(N^2), but r154053 reversed these making the more common nth-child slower. It included an optimization to make both O(1), but the optimization was broken in one of the merges. I will re-land the working optimization in another patch, and with a magnitude perf test Ojan, eseidel, and myself wrote to ensure it works. R=ojan@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=154072

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -37 lines) Patch
M Source/core/css/SelectorChecker.h View 3 chunks +1 line, -3 lines 0 comments Download
M Source/core/css/SelectorChecker.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/css/resolver/StyleResolver.h View 1 chunk +1 line, -7 lines 0 comments Download
M Source/core/css/resolver/StyleResolver.cpp View 4 chunks +4 lines, -4 lines 0 comments Download
M Source/core/css/resolver/StyleResolverState.h View 3 chunks +2 lines, -5 lines 0 comments Download
M Source/core/css/resolver/StyleResolverState.cpp View 3 chunks +3 lines, -4 lines 0 comments Download
M Source/core/dom/Element.h View 2 chunks +3 lines, -3 lines 0 comments Download
M Source/core/dom/Element.cpp View 5 chunks +7 lines, -9 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
leviw_travelin_and_unemployed
7 years, 5 months ago (2013-07-12 01:14:03 UTC) #1
ojan
lgtm
7 years, 5 months ago (2013-07-12 01:19:30 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/leviw@chromium.org/18147020/1
7 years, 5 months ago (2013-07-12 01:19:56 UTC) #3
eseidel
7 years, 5 months ago (2013-07-12 03:50:24 UTC) #4
Message was sent while issue was closed.
Committed patchset #1 manually as r154072 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698