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

Issue 108723007: Introduce TreeScopeEventContext (Closed)

Created:
7 years ago by hayato
Modified:
6 years, 11 months ago
Reviewers:
dglazkov, eseidel
CC:
blink-reviews
Visibility:
Public.

Description

Introduce TreeScopeEventContext. Some information can be shared between EventContexts whose TreeScope is the same. Instead of storing such information in each EventContext (which is created per Node), store such information in newly introduced TreeScopeEventContext (which is created per TreeScope). Though it might take some computation cost to share such information, the pros is much bigger than the cons. We have to traverse only treeScopes, rather than nodes, when calculating some information, such as adjusted relatedTarget, adjusted event path, adjusted touch targets and so on. We can get benefits both in time and space as follows: 1. Time The result of the benchmarks are: PerformanceTests/Events/EventDispatching.html 541 runs/s vs 594 runs/s (10% better score) PerformanceTests/Events/EventDispatchingInShadowTrees.html 4625 runs/s vs 5741 runs/s (24% better score) 2. Space We can reduce the memory footprint about 1.5 KBytes per an event. BUG=329445 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=164501

Patch Set 1 #

Total comments: 1

Patch Set 2 : Rename SharedEventContext to TreeScopeEventContext #

Unified diffs Side-by-side diffs Delta from patch set Stats (+310 lines, -315 lines) Patch
A PerformanceTests/Events/EventsDispatchingInShadowTrees.html View 1 chunk +66 lines, -0 lines 0 comments Download
M Source/core/core.gypi View 1 1 chunk +0 lines, -2 lines 0 comments Download
M Source/core/events/EventContext.h View 1 3 chunks +41 lines, -19 lines 0 comments Download
M Source/core/events/EventContext.cpp View 1 2 chunks +25 lines, -9 lines 0 comments Download
M Source/core/events/EventDispatcher.cpp View 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/events/EventPath.h View 1 4 chunks +22 lines, -3 lines 0 comments Download
M Source/core/events/EventPath.cpp View 1 5 chunks +149 lines, -20 lines 0 comments Download
D Source/core/events/EventRetargeter.h View 1 chunk +0 lines, -64 lines 0 comments Download
D Source/core/events/EventRetargeter.cpp View 1 chunk +0 lines, -187 lines 0 comments Download
M Source/core/events/FocusEvent.cpp View 5 chunks +4 lines, -5 lines 0 comments Download
M Source/core/events/MouseEvent.cpp View 3 chunks +2 lines, -3 lines 0 comments Download
M Source/core/events/TouchEvent.cpp View 2 chunks +1 line, -2 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
hayato
PTAL. I am not in a hurry. :) Enjoy your vacation.
7 years ago (2013-12-24 08:52:36 UTC) #1
dglazkov
LGTM, this looks great. https://codereview.chromium.org/108723007/diff/1/Source/core/events/EventContext.h File Source/core/events/EventContext.h (right): https://codereview.chromium.org/108723007/diff/1/Source/core/events/EventContext.h#newcode58 Source/core/events/EventContext.h:58: class SharedEventContext : public RefCounted<SharedEventContext> ...
6 years, 12 months ago (2013-12-26 17:30:13 UTC) #2
hayato
Thank you for the review. On 2013/12/26 17:30:13, Dimitri Glazkov wrote: > LGTM, this looks ...
6 years, 11 months ago (2014-01-06 07:33:47 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/hayato@chromium.org/108723007/140001
6 years, 11 months ago (2014-01-06 08:30:35 UTC) #4
commit-bot: I haz the power
Change committed as 164501
6 years, 11 months ago (2014-01-06 10:13:07 UTC) #5
hayato
6 years, 11 months ago (2014-01-06 10:51:15 UTC) #6
Message was sent while issue was closed.
On 2014/01/06 10:13:07, I haz the power (commit-bot) wrote:
> Change committed as 164501

It seems that I forgot to rename some member variables and parameter names.
Let me do that in a follow-up patch.

Powered by Google App Engine
This is Rietveld 408576698