I did a quick and dirty implementation of this:
var entry = {key: UNDEFINED, value: UNDEFINED};
while (%MapIteratorNextWithEntry(iterator, entry)) {
%_CallFunction(receiver, entry.value, entry.key, this, f);
}
and the result is:
MapIteratorNext (master)
Map-Collections(Score): 65281
Map-Collections(Score): 65271
Map-Collections(Score): 65480
MapIteratorNextWithEntry:
Map-Collections(Score): 69872
Map-Collections(Score): 69966
Map-Collections(Score): 70161
MapIteratorHasMore + MapIteratorCurrentKey + MapIteratorCurrentValue +
MapIteratorMoveNext
Map-Collections(Score): 82950
Map-Collections(Score): 82252
Map-Collections(Score): 81031
So, even with those 3 extra runtime calls it is much faster.
On Mon, Jun 23, 2014 at 6:11 PM, Erik Arvidsson <[email protected]> wrote:
> On Mon, Jun 23, 2014 at 6:04 PM, <[email protected]> wrote:
>
>> On 2014/06/23 21:00:04, arv wrote:
>>
>>> On 2014/06/23 at 20:47:27, danno wrote:
>>> > On 2014/06/23 19:51:52, Michael Starzinger wrote:
>>> > > Handing off to either Andreas or Danno.
>>> > >
>>> > > https://codereview.chromium.org/329253004/diff/90001/src/
>>> collection.js
>>> > > File src/collection.js (right):
>>> > >
>>> > >
>>>
>>
>> https://codereview.chromium.org/329253004/diff/90001/src/
>> collection.js#newcode87
>>
>>> > > src/collection.js:87: %SetIteratorMoveNext(iterator);
>>> > > I am _very_ surprised that this is faster. How are three runtime
>>> calls
>>> faster
>>> > > than one? Don't get me wrong, I trust your measurements, but is this
>>> a
>>>
>> good
>>
>>> > > optimization in the long run?
>>> >
>>> > NOT LGTM
>>> >
>>> > Replacing one runtime function with three might provide a win in some
>>> situations (how did you measure it?),
>>>
>>
>> Using the tests that we added to golem
>>>
>>
>> > but I am a concerned that this is not the way to go if you are wanting
>>> to
>>> optimize this for the long run. If you really want a fast
>>> implementation, you
>>> long-term aim should be to remove the runtime call altogether (which is
>>> possible, e.g. with a Hydrogen stub, but obviously more work).
>>>
>>
>> We have explored this path. There was a concern that we should not write
>>> hydrogen for too specialized code. And I also wrote a self hosted
>>> version of
>>>
>> Map
>>
>>> and Set where the only runtime call was to get the hash code. This,
>>> however
>>> turned out to be a lot slower so it was decided not to pursue this path
>>> for
>>>
>> now.
>>
>> I am not sure if the concern is with hydrogen stubs in general for
>> optimization
>> here, I would rather frame it this way: code generation is not a very
>> subtle
>> tool and should be used sparingly and as the final step in the
>> optimization
>> process. Before you consider Hydrogen stub generation, the other
>> bottlenecks
>> should be reduced and massaged so that when the last little bit needs to
>> be
>> squeezed out with code generation, it can be done minimally intrusively.
>> Given
>> we just started with the optimization process, let's focus on the
>> low-hanging
>> fruits first (as you have conceptually done), but not design out the path
>> to the
>> high-hanging ones like Hydrogen stubs eventually. The means incrementally
>> teasing out more and more functionality of the native runtime call in
>> JavaScript
>> (which get's turned into highly optimized code via Crankshaft "for free")
>> so
>> that the code that remains in C++ is easy and concise to implement in
>> Hydrogen.
>>
>>
>>
>>
>> > Your proposed solution relies too much on C++ machinery in order to
>>> make
>>> forEach faster and moves further away from the ideal solution, so it
>>> doesn't
>>> really match current v8 practice.
>>> >
>>> > What exactly is causing the existing function to be slow? Did you
>>> measure
>>>
>> it?
>>
>>> Is it the allocation for every iteration? Or something else?
>>>
>>
>> > If it's the allocation, I can think of another, simpler ways to improve
>>>
>> this.
>>
>>> For example, this particular use case doesn't expose the entry object
>>> externally, so a _much_ faster approach is to turn %XXXIteratorNext into
>>> %XXXIteratorNextObject (or whatever appropriate name) that either
>>> returns the
>>> next object, or a special sentinel for done. You can call that new
>>> version
>>> directly from the forEach implementation, removing all allocation and
>>> doing
>>>
>> the
>>
>>> job with only a single runtime function.
>>> >
>>> > If you do it right, I think you might be able get rid of the nastiness
>>> with
>>> the special casing of the entry object and its map in the runtime,
>>> turning the
>>> creating of the externally-visible entry into pure JS something like
>>> this:
>>> >
>>> > function SetIteratorNext() {
>>> > var next = %SetIteratorNextObject();
>>> > var is_done = next == __some_sentinel;
>>> > return { value: (is_done ? undefined : next), done: is_done }
>>> > }
>>>
>>
>> Thanks for the feedback Danno. Much appreciated.
>>>
>>
>> The sentinel solution works for Set.prototype.forEach but not for
>>> Map.prototype.forEach which needs both key and value.
>>>
>>
>> Yes, but consider this variant of that approach:
>>
>> function MapIteratorNext() {
>> var entry = { key: undefined, value: undefined, done: false };
>> %MapIteratorNextWithEntry(iterator, entry);
>> }
>>
>> In the forEach case, you can allocate a single entry and pass it into
>> multiple
>> calls of the %MapIteratorNextWithEntry, reducing the overhead (one
>> allocation
>> vs. n)
>
>
> This approached occurred to me as well. Let me try to see how it turns out.
>
>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> https://codereview.chromium.org/329253004/
>>
>> --
>> --
>> 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.
>>
>
>
>
> --
> erik
>
>
>
--
erik
--
--
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.
