PTAL
https://codereview.chromium.org/48913008/diff/100001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/48913008/diff/100001/src/objects.cc#newcode15764
src/objects.cc:15764: new_table->set(table->EntryToIndex(entry), *key);
That makes sense. The enclosing methods are now static as well, so these
calls are now explicitly namespaced with the classname (e.g.
ObjectHashSet::EntryToIndex, in this case).
On 2013/11/04 13:46:42, Michael Starzinger wrote:
On 2013/11/04 10:58:06, rafaelw wrote:
> One question, this probably doesn't matter, but it seemed strange to
me that
the
> original code was:
>
> table->set(EntryToIndex(entry), key);
>
> seems like it should be
>
> table->set(table->EntryToIndex(entry), key);
>
> or here,
>
> new_table->set(new_table->EntryToIndex(entry), *key);
Hmm, HashTable::EntryToIndex() should be a static function. It
shouldn't need a
receiver object.
https://codereview.chromium.org/48913008/diff/100001/src/objects.h
File src/objects.h (right):
https://codereview.chromium.org/48913008/diff/100001/src/objects.h#newcode1515
src/objects.h:1515: static Handle<Object> GetHash(Handle<Object> object,
Isolate* isolate,
On 2013/11/04 10:29:11, Michael Starzinger wrote:
nit: The current implementation as a wrapper requires the isolate
parameter.
Once the implementation is fully handlified the isolate parameter is
no longer
needed. Let's add a TODO to keep that in mind.
Done (although this todo is now on GetOrCreateHash
https://codereview.chromium.org/48913008/diff/100001/src/objects.h#newcode4066
src/objects.h:4066: static bool Contains(Handle<ObjectHashSet> table,
Handle<Object> key);
On 2013/11/04 10:29:11, Michael Starzinger wrote:
The ObjectHashSet::Contains method should never cause an allocation,
it should
be possible to leave this one un-handlified.
Done.
https://codereview.chromium.org/48913008/diff/100001/src/objects.h#newcode4068
src/objects.h:4068: static Handle<ObjectHashSet> EnsureSetCapacity(
Done. Applied to ObjectHash(Set/Table)::(EnsureCapacity/Shrink)
On 2013/11/04 10:29:11, Michael Starzinger wrote:
On 2013/10/29 19:14:51, rafaelw wrote:
> Note that I've named this "EnsureSetCapacity" because I can't figure
out how
to
> get the C++ compiler to stop complaining because in the
CALL_HEAP_FUNCTION
> implementation, it seems to be trying to invoke the static method
(forgetting
> about the inherted templatized method from HashTable.
>
> I'm hoping you know what the solution to this is. Same problem below
with
> ShrinkSet.
One way to address that I can think of would be to add an explicit
up-cast into
the implementation of ObjectHashSet::EnsureSetCapacity like this ...
Handle<HashTable<ObjectHashTableShape<1>, Object*> > table_base =
table;
Unfortunately this will lead to access violations because
HashTable::EnsureSetCapacity is protected. But I think we can bake
ObjectHashSet
a friend of HashTable to resolve this.
https://codereview.chromium.org/48913008/diff/100001/test/cctest/test-dictionary.cc
File test/cctest/test-dictionary.cc (right):
https://codereview.chromium.org/48913008/diff/100001/test/cctest/test-dictionary.cc#newcode182
test/cctest/test-dictionary.cc:182: // Calling Remove() should not cause
GC ever.
On 2013/10/29 19:14:51, rafaelw wrote:
This comment doesn't seem true. I believe it's always true for this
test, but
Remove() can clearly allocate more memory via Shrink.
Done.
https://codereview.chromium.org/48913008/diff/170002/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/48913008/diff/170002/src/objects.cc#newcode1047
src/objects.cc:1047: ASSERT(IsJSReceiver());
Note that I've changed this for consistency with GetOrCreateHash below,
which cannot return Smi::FromInt(0) after an unreachable, because no
Isolate is available at that point to create a handle.
https://codereview.chromium.org/48913008/diff/170002/src/objects.cc#newcode4808
src/objects.cc:4808: Handle<Object> hash =
JSObject::GetOrCreateIdentityHash(object);
I *think* this is right. It's a bit hard for me to follow the
CALL_AND_RETRY_OR_DIE logic. Please verify.
https://codereview.chromium.org/48913008/
--
--
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/groups/opt_out.
