As mentioned in the comment there is no need to "pre-allocate" symbols for strings used as property names.
http://codereview.chromium.org/1694011/diff/35007/38001 File src/api.cc (right): http://codereview.chromium.org/1694011/diff/35007/38001#newcode1535 src/api.cc:1535: i::Handle<i::JSObject> obj(i::JSObject::cast(self->GetElement(index))); On 2010/05/03 19:16:02, jaimeyap wrote:
On 2010/05/03 07:59:21, Søren Gjesse wrote: > Please add a range check on index and return undefined if out of
bounds.
Doesn't GetElement() already return undefined if index doesn't match
the range? Thats right. http://codereview.chromium.org/1694011/diff/35007/38003 File src/top.cc (right): http://codereview.chromium.org/1694011/diff/35007/38003#newcode385 src/top.cc:385: Handle<String> column_key = Factory::NewStringFromAscii( On 2010/05/03 19:16:02, jaimeyap wrote:
On 2010/05/03 07:59:21, Søren Gjesse wrote: > Use Factory::LookupAsciiSymbol(kXXX) here.
Hmm. But in order to use LookupAsciiSymbol I would need to
pre-allocate all the
key symbols and add them to the roots_ array in heap.h.
Do we want to always allocate these Symbols for the common case? Seems
like it
would be better to only pay the penalty when you grab a stack trace.
If you still think I should do this, let me know and I will put these
symbols
there.
Thats not right. Factory::LookupAsciiSymbol will create the symbol if it is not found in the symbol table already. Also when using the string as a property name a symbol with this string will be created anyway. Using a symbol to begin with then saves SetProperty from looking it up each time. http://codereview.chromium.org/1694011/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
