|
|
Created:
4 years, 3 months ago by kochi Modified:
4 years, 3 months ago Reviewers:
dominicc (has gone to gerrit) CC:
blink-reviews, blink-reviews-dom_chromium.org, chromium-reviews, dglazkov+blink, dominicc+watchlist_chromium.org, eae+blinkwatch, rwlbuis, sof, webcomponents-bugzilla_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAvoid redundant hash calculations in CustomElementUpgradeSorter
Pointed out by tkent in https://codereview.chromium.org/2242743002/
The original code could do 2 hash calculations for
adding a new key-value pair to ParentChildMap
(for find() and add()), and rehashing the map
may involve copying the HeapHashSet<Member<Node>>
objects.
Change the value part of ParentChildMap from a HeapHashMap
object to its pointer, to avoid redundant hash calculation
and object copy.
This does not change any functionality of the class.
BUG=594918
Committed: https://crrev.com/130931a943430f64f69dfb238571d74762f84160
Cr-Commit-Position: refs/heads/master@{#415251}
Patch Set 1 #
Messages
Total messages: 19 (12 generated)
Description was changed from ========== Avoid redundant hash calculations in CustomElementUpgradeSorter.cpp Pointed out by tkent @ https://codereview.chromium.org/2242743002/diff/490001/third_party/WebKit/Sou... The original code had 2 hash calculations for key lookup (for find() and add()), and possibly big memory of for HeapHashSet<Member<Node>> when rehash happens. Change the value part of ParentChildMap from a HeapHashMap object to its pointer, to avoid redundant hash calculation and object copy. This does not change any functionality of the class. BUG=594918 ========== to ========== Avoid redundant hash calculations in CustomElementUpgradeSorter.cpp Pointed out by tkent in https://codereview.chromium.org/2242743002/ The original code had 2 hash calculations for key lookup (for find() and add()), and possibly big memory of for HeapHashSet<Member<Node>> when rehash happens. Change the value part of ParentChildMap from a HeapHashMap object to its pointer, to avoid redundant hash calculation and object copy. This does not change any functionality of the class. BUG=594918 ==========
kochi@chromium.org changed reviewers: + dominicc@chromium.org
The CQ bit was checked by kochi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Avoid redundant hash calculations in CustomElementUpgradeSorter.cpp Pointed out by tkent in https://codereview.chromium.org/2242743002/ The original code had 2 hash calculations for key lookup (for find() and add()), and possibly big memory of for HeapHashSet<Member<Node>> when rehash happens. Change the value part of ParentChildMap from a HeapHashMap object to its pointer, to avoid redundant hash calculation and object copy. This does not change any functionality of the class. BUG=594918 ========== to ========== Avoid redundant hash calculations in CustomElementUpgradeSorter Pointed out by tkent in https://codereview.chromium.org/2242743002/ The original code had 2 hash calculations for key lookup (for find() and add()), and possibly big memory of for HeapHashSet<Member<Node>> when rehash happens. Change the value part of ParentChildMap from a HeapHashMap object to its pointer, to avoid redundant hash calculation and object copy. This does not change any functionality of the class. BUG=594918 ==========
Hi Dominic, This was pointed out by tkent @ https://codereview.chromium.org/2242743002/diff/490001/third_party/WebKit/Sou... which was in the original code. Could you review?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Exceeded global retry quota
On 2016/08/29 at 07:35:09, kochi wrote: > Hi Dominic, > > This was pointed out by tkent @ > https://codereview.chromium.org/2242743002/diff/490001/third_party/WebKit/Sou... > which was in the original code. > > Could you review? Yes, this is nice. I thought heap collections had some special magic for members that were heap collections, but I just looked at the documentation and the section on heap collections appears to have been deleted. Can you fix the description? "possibly big memory of for HeapHashSet<Member<Node>> when rehash happens." in particular needs work.
Description was changed from ========== Avoid redundant hash calculations in CustomElementUpgradeSorter Pointed out by tkent in https://codereview.chromium.org/2242743002/ The original code had 2 hash calculations for key lookup (for find() and add()), and possibly big memory of for HeapHashSet<Member<Node>> when rehash happens. Change the value part of ParentChildMap from a HeapHashMap object to its pointer, to avoid redundant hash calculation and object copy. This does not change any functionality of the class. BUG=594918 ========== to ========== Avoid redundant hash calculations in CustomElementUpgradeSorter Pointed out by tkent in https://codereview.chromium.org/2242743002/ The original code had 2 hash calculations for key lookup (for find() and add()), and rehash may involve copying the whole HeapHashSet<Member<Node>> contents. Change the value part of ParentChildMap from a HeapHashMap object to its pointer, to avoid redundant hash calculation and object copy. This does not change any functionality of the class. BUG=594918 ==========
Description was changed from ========== Avoid redundant hash calculations in CustomElementUpgradeSorter Pointed out by tkent in https://codereview.chromium.org/2242743002/ The original code had 2 hash calculations for key lookup (for find() and add()), and rehash may involve copying the whole HeapHashSet<Member<Node>> contents. Change the value part of ParentChildMap from a HeapHashMap object to its pointer, to avoid redundant hash calculation and object copy. This does not change any functionality of the class. BUG=594918 ========== to ========== Avoid redundant hash calculations in CustomElementUpgradeSorter Pointed out by tkent in https://codereview.chromium.org/2242743002/ The original code could do 2 hash calculations for adding a new key-value pair to ParentChildMap (for find() and add()), and rehashing the map may involve copying the HeapHashSet<Member<Node>> objects. Change the value part of ParentChildMap from a HeapHashMap object to its pointer, to avoid redundant hash calculation and object copy. This does not change any functionality of the class. BUG=594918 ==========
Thanks for the review! I updated the description. Could you take another look?
The CQ bit was checked by dominicc@chromium.org
lgtm Very nice!
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Avoid redundant hash calculations in CustomElementUpgradeSorter Pointed out by tkent in https://codereview.chromium.org/2242743002/ The original code could do 2 hash calculations for adding a new key-value pair to ParentChildMap (for find() and add()), and rehashing the map may involve copying the HeapHashSet<Member<Node>> objects. Change the value part of ParentChildMap from a HeapHashMap object to its pointer, to avoid redundant hash calculation and object copy. This does not change any functionality of the class. BUG=594918 ========== to ========== Avoid redundant hash calculations in CustomElementUpgradeSorter Pointed out by tkent in https://codereview.chromium.org/2242743002/ The original code could do 2 hash calculations for adding a new key-value pair to ParentChildMap (for find() and add()), and rehashing the map may involve copying the HeapHashSet<Member<Node>> objects. Change the value part of ParentChildMap from a HeapHashMap object to its pointer, to avoid redundant hash calculation and object copy. This does not change any functionality of the class. BUG=594918 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Avoid redundant hash calculations in CustomElementUpgradeSorter Pointed out by tkent in https://codereview.chromium.org/2242743002/ The original code could do 2 hash calculations for adding a new key-value pair to ParentChildMap (for find() and add()), and rehashing the map may involve copying the HeapHashSet<Member<Node>> objects. Change the value part of ParentChildMap from a HeapHashMap object to its pointer, to avoid redundant hash calculation and object copy. This does not change any functionality of the class. BUG=594918 ========== to ========== Avoid redundant hash calculations in CustomElementUpgradeSorter Pointed out by tkent in https://codereview.chromium.org/2242743002/ The original code could do 2 hash calculations for adding a new key-value pair to ParentChildMap (for find() and add()), and rehashing the map may involve copying the HeapHashSet<Member<Node>> objects. Change the value part of ParentChildMap from a HeapHashMap object to its pointer, to avoid redundant hash calculation and object copy. This does not change any functionality of the class. BUG=594918 Committed: https://crrev.com/130931a943430f64f69dfb238571d74762f84160 Cr-Commit-Position: refs/heads/master@{#415251} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/130931a943430f64f69dfb238571d74762f84160 Cr-Commit-Position: refs/heads/master@{#415251} |