Reviewers: arv, Sven Panne,
Message:
Thanks for taking a look!
https://codereview.chromium.org/947683002/diff/20001/src/collection.js
File src/collection.js (right):
https://codereview.chromium.org/947683002/diff/20001/src/collection.js#newcode218
src/collection.js:218: while (%SetIteratorNext(iterator, value_array)) {
On 2015/02/21 16:20:16, arv wrote:
This one can get much faster now that we have access to the underlying
elements.
Lets revisit.
Yeah, there are definitely opportunities to speed up iterators too, but
this patch is already huge :)
https://codereview.chromium.org/947683002/diff/20001/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/947683002/diff/20001/src/hydrogen.cc#newcode12104
src/hydrogen.cc:12104: void
HOptimizedGraphBuilder::GenerateFixedArraySetTheHole(CallRuntime* call)
{
On 2015/02/23 09:26:53, Sven Panne wrote:
Hmmm, perhaps a %_Hole() intrinsic is nicer, you could use a
combination of this
with the %_FixedArraySet intrinsic above then. As it is, this looks a
bit too
specialized.
Yeah, I should have left a TODO on this.
https://codereview.chromium.org/947683002/diff/20001/src/hydrogen.cc#newcode12117
src/hydrogen.cc:12117: void
HOptimizedGraphBuilder::GenerateJSCollectionGetTable(CallRuntime* call)
{
On 2015/02/23 09:26:53, Sven Panne wrote:
We can leave it as it is for now, but I think that in the future we
will have
something more general like %_GetInternalField.
So this is an interesting one. The Hydrogen version would actually have
no problem with %_GetInternalField, but I ran into a lot of existing
Runtime code that tries to be safe about accessing internal fields.
Given that I only needed access to a single internal field in this
patch, this seemed the path of least resistance. But I agree that the
real version should probably not expose this.
https://codereview.chromium.org/947683002/diff/20001/src/hydrogen.cc#newcode12151
src/hydrogen.cc:12151: IfBuilder string_checker(this);
On 2015/02/23 09:26:53, Sven Panne wrote:
As a general rule of thumb, we should not have any non-trivial control
flow in
intrinsics. This should be expressed on the JavaScript level.
Thanks, this is good feedback. "No non-trivial control flow" is a
guideline I didn't have in mind for these, but now I do.
https://codereview.chromium.org/947683002/diff/20001/src/hydrogen.h
File src/hydrogen.h (right):
https://codereview.chromium.org/947683002/diff/20001/src/hydrogen.h#newcode1852
src/hydrogen.h:1852: HValue* BuildElementIndexHash(HValue* index) {
On 2015/02/23 09:26:53, Sven Panne wrote:
When we have %_HashSeed() (for isolate()->heap()->HashSeed()), this
can be
expressed purely in JavaScript. As it is, it is too specialized.
Yeah, I only reused this for expediency, there's no reason we couldn't
compute Smi hashes in JS (String hashes are more complex, and I think
there are more tradeoffs there).
https://codereview.chromium.org/947683002/diff/20001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/947683002/diff/20001/src/objects.cc#newcode810
src/objects.cc:810: return Smi::FromInt(FastD2I(num))->GetHash();
On 2015/02/21 16:20:16, arv wrote:
Why not call ComputeIntegerHash here directly so we do not have to
check if it
is a Smi again?
Laziness, happy to clean this up.
Description:
WORK IN PROGRESS: Reimplement Maps and Sets in JS
DO NOT LAND
Please review this at https://codereview.chromium.org/947683002/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+553, -1170 lines):
M src/collection.js
M src/hydrogen.h
M src/hydrogen.cc
M src/macros.py
M src/objects.h
M src/objects.cc
M src/runtime.js
M src/runtime/runtime.h
M src/runtime/runtime-collections.cc
M test/cctest/cctest.gyp
M test/cctest/test-dictionary.cc
D test/cctest/test-ordered-hash-table.cc
M test/js-perf-test/Collections/run.js
--
--
v8-dev mailing list
[email protected]
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, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
