https://codereview.chromium.org/742643003/diff/1/src/harmony-templates.js
File src/harmony-templates.js (right):
https://codereview.chromium.org/742643003/diff/1/src/harmony-templates.js#newcode24
src/harmony-templates.js:24: var obj = callSiteCache.get(hash);
On 2014/11/19 06:09:11, arv wrote:
This needs to be be %MapGet(callSiteCache, hash) or someone can hijack
this.
Done.
https://codereview.chromium.org/742643003/diff/1/src/harmony-templates.js#newcode28
src/harmony-templates.js:28: if ("raw" in obj) {
On 2014/11/19 06:09:11, arv wrote:
On 2014/11/19 03:50:33, caitp wrote:
> I know this is broken if Array.prototype is messed with. Maybe a
private
symbol
> to identify these would make sense?
Why is this needed. Can we not make sure we only store valid call site
objects
in the cache?
In an unlikely event where we have hash collisions, the value is a List
of some kind, rather than a single callSiteObj. The lone object is a
valid call site, it's just to determine if it's an array of callsites,
or an array of strings (single callsite).
Less-complex alternative: Always put cached values in an array, even
when they're alone (wastes a small bit of memory for the common
no-collision case).
https://codereview.chromium.org/742643003/diff/1/src/harmony-templates.js#newcode49
src/harmony-templates.js:49: } else if (IS_ARRAY(obj)) {
On 2014/11/19 06:09:11, arv wrote:
How can obj be anything but an array?
Explained above --- it can be a callSiteObj, or an array of callSiteObjs
--- callSiteObj is an array, so it's a bit confusing and bad.
Alternative was already proposed, if it sounds good I don't see why not.
https://codereview.chromium.org/742643003/diff/1/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/742643003/diff/1/src/parser.cc#newcode5333
src/parser.cc:5333: Vector<uint8_t> hash_string =
Vector<uint8_t>::New(num_dummy_chars);
On 2014/11/19 06:09:11, arv wrote:
What about two byte strings?
String::ToCString claims to return a UTF8 representation of the string,
so whether it's got multi-byte characters or not, I don't think it
should matter.
Can we use StringHasher::HashSequentialString here?
I didn't know there was a hashing algorithm built in, I'll use that
instead of Murmur.
https://codereview.chromium.org/742643003/
--
--
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.
