Thanks a lot for review, Christian. I'm planning to kill this CL in favour of http://codereview.chromium.org/255004/show , so most of responds refer to the newer CL.
http://codereview.chromium.org/248031/diff/3001/3002 File src/api.cc (right): http://codereview.chromium.org/248031/diff/3001/3002#newcode2448 Line 2448: // TODO(antonm): consider some fancier ways (like Duff's). On 2009/09/30 05:26:57, Christian Plesner Hansen wrote: > Technically, yes -- call it something Duff's-device-like then. Another > possibility I discussed with Erik was reading two chars at a time and using > binary operations to construct a whole word at a time to write into the target > buffer. In newer patch I just switched to CopyChars so all the optimizations could benefit other parts of V8. Just FYI: I attempted to special case CopyChars to use memcpy and that didn't boost us. I didn't investigate why though. http://codereview.chromium.org/248031/diff/3001/3002#newcode2478 Line 2478: bool copy_eos = end < length; On 2009/09/30 05:26:57, Christian Plesner Hansen wrote: > Shouldn't you always copy_eos at this level? In the functions above you > transfer characters from 0 to end-1 for end<=length, and the eos is at position > |length|. > Nit: copy_eos is a misleading name because the eos isn't copied. Agree, that hopefully shouldn't be an issue for a new patch. http://codereview.chromium.org/248031/diff/3001/3002#newcode2522 Line 2522: } else if (second->length() == 1) { On 2009/09/30 05:26:57, Christian Plesner Hansen wrote: > What motivates this special case? Just FYI: that's a common case in one Dromaeo test. Once again in the newer patch there is no need in such special casing. http://codereview.chromium.org/248031 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
