Cool stuff! Mostly nits:
https://codereview.chromium.org/1316213008/diff/80001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/1316213008/diff/80001/src/objects.cc#newcode6492
src/objects.cc:6492: int KeyAccumulator::GetLength() { return length_; }
I think it's
Lgtm with a couple of nits:
https://codereview.chromium.org/1316213008/diff/160001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/1316213008/diff/160001/src/objects.cc#newcode6610
src/objects.cc:6610: int KeyAccumulator::GetLength() { return length_; }
Please move
addressing nits
https://codereview.chromium.org/1316213008/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1316213008/180001
View timeline at
https://chromium-cq-status.appspot.com/patch-timeline/1316213008/180001
https://codereview.chromium.org/1316213008/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
adressed your feedback
https://codereview.chromium.org/1316213008/diff/140001/src/elements.cc
File src/elements.cc (right):
https://codereview.chromium.org/1316213008/diff/140001/src/elements.cc#newcode876
src/elements.cc:876: virtual void
AddElementsToFixedArrayWithAccumulator(
On 2015/09/17
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1316213008/21
View timeline at
https://chromium-cq-status.appspot.com/patch-timeline/1316213008/21
https://codereview.chromium.org/1316213008/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
Patchset 11 (id:??) landed as
https://crrev.com/007eac94a14f5b4db36babeaa1e8fd677aec2c17
Cr-Commit-Position: refs/heads/master@{#30797}
https://codereview.chromium.org/1316213008/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this
Committed patchset #11 (id:21)
https://codereview.chromium.org/1316213008/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group
PTAL
Updated CL with OrderedHashSet
https://codereview.chromium.org/1316213008/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group
https://codereview.chromium.org/1316213008/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it,
No L-G-T-M from a valid reviewer yet. Only full committers are accepted.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.
https://codereview.chromium.org/1316213008/diff/80001/src/objects.h
File src/objects.h (right):
https://codereview.chromium.org/1316213008/diff/80001/src/objects.h#newcode10471
src/objects.h:10471: class KeyAccumulator final BASE_EMBEDDED {
I wasn't really sure where to put the class
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1316213008/80001
View timeline at
https://chromium-cq-status.appspot.com/patch-timeline/1316213008/80001
https://codereview.chromium.org/1316213008/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
PTAL
https://codereview.chromium.org/1316213008/diff/60001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/1316213008/diff/60001/src/objects.cc#newcode6561
src/objects.cc:6561: if (length_ * count < 100) return;
I tried a bit with different values and 100 turned out
Reviewers: Igor Sheludko,
Message:
PTAL
I think some more work could be done, but I am lacking the proper data
structures.
Something like a SortedHashSet would be perfect, but it seems like the
existing
implementation doesn't expose a proper C++ interface (those used by JSSet
and
JSMap).
15 matches
Mail list logo
