LGTM.
http://codereview.chromium.org/1563005/diff/9012/25009 File src/runtime.cc (right): http://codereview.chromium.org/1563005/diff/9012/25009#newcode10022 src/runtime.cc:10022: cache->set(kCacheSizeIndex, Smi::FromInt(kEntriesIndex)); My mistake. I was confusing the two meanings of size (initial capacity and currently used capacity) so I thought this field was holding the capacity, not the next free entry pointer. http://codereview.chromium.org/1563005/diff/9012/25009#newcode10079 src/runtime.cc:10079: // Search the cache in less recently used order. Maybe something to say that LRU only holds as long as we miss. I'm worrying that if you alternate between two input values, every other finger-miss will take the maximal time to find the match. And using a new key at that point is 50% likely to overwrite the second-most-recently-used entry. On the other hand, we don't want to reorder the cache based on usage, so we only track the most recently used element (the finger) and don't really know what the second most recently used element was. I guess there isn't any simple way to do better. http://codereview.chromium.org/1563005/diff/9012/25011 File src/string.js (right): http://codereview.chromium.org/1563005/diff/9012/25011#newcode519 src/string.js:519: %NewCache(0, 16, function(re) { return new $RegExp(re); }); Not sure why you couldn't use $RegExp, but a guess would be that it didn't exist yet at the time the natives code file was loaded. Our natives code files really contain two parts: the runtime functions and the initialization time setup. The setup cannot rely on another natives file having been loaded before it, but the runtime functions are only called when all natives files have been loaded and initialized. In this case, global.RegExp was created by the bootstrapper code before loading natives, but $RegExp isn't created until regexp.js is loaded (and global.RegExp doesn't work until that has happened either, because regexp.js sets the code of the function). http://codereview.chromium.org/1563005/diff/9012/25012 File test/mjsunit/fuzz-natives.js (right): http://codereview.chromium.org/1563005/diff/9012/25012#newcode164 test/mjsunit/fuzz-natives.js:164: // additional checks. Maybe add something saying that the function only accepts a Smi literal as first argument, which is checked at compile time. That's the real reason we can't test it properly with fuzz-natives (right?). http://codereview.chromium.org/1563005/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev To unsubscribe, reply using "remove me" as the subject.
