Optimize toPrimitive / toNumber conversions for 0.5% gain on SunSpider
Created attachment 16871 [details] patch
Comment on attachment 16871 [details] patch There must be some script for generating JavaScriptCore.exp, since I see you reordering the symbol I put in the wrong place (maybe you just used sort..) I don't know why GetterSetterImp's ASSERT on primative calls, but I haven't looked at the code, looks like the other primative calls do too. wasNotString1 would make more sense to me as v1IsNumber or something, or n1isValid, or something non-negative. JSObject::getPrimativeNumber seems overly complicated. I guess defaultValues for primitives can change? (I can just go look at the code...) Hum... isn't this wrong? + if (JSImmediate::isImmediate(this)) { + number = JSImmediate::toDouble(this); it should be isNumber(this) no? Otherwise you'll call toDouble on jsUndefined(), no? Assuming you've run-javascriptcore-tests and fast/js (and you answer my unsigned query), this is fine. I can mark it r+ when you reappear on IRC.
jsUndefined() query, rather. Gosh I'm tired.
getPrimitiveNumber() never fails. It *always* converts to number. The boolean return does not indicate whether it succeeded. It only indicates that this value did *not* have a preference for turning into a string instead of a number when converting toPrimitive() with a number hint. (I realize that's a little fruity, but that is pretty much the rule that less and lessEq require.) To address specific points: (In reply to comment #2) > (From update of attachment 16871 [details] [edit]) > There must be some script for generating JavaScriptCore.exp, since I see you > reordering the symbol I put in the wrong place (maybe you just used sort..) I used M-x sort-lines in emacs. > I don't know why GetterSetterImp's ASSERT on primative calls, but I haven't > looked at the code, looks like the other primative calls do too. GetterSetterImp only exists to have a garbage-collectable thing to put in the property map. It is never exposed to JavaScript, so the toPrimitive and related conversions can never actually be called in practice. > wasNotString1 would make more sense to me as v1IsNumber or something, or > n1isValid, or something non-negative. The problem here is that the meaning is weird. All it says is that the value that was converted *didn't* prefer string. Maybe v1PrefersNumber? > JSObject::getPrimativeNumber seems overly complicated. I guess defaultValues > for primitives can change? (I can just go look at the code...) It's just doing the combination of what JSObject::toPrimitive() and toNumber() on the result would do (plus checking whether a string was preferred). > Hum... isn't this wrong? > > + if (JSImmediate::isImmediate(this)) { > + number = JSImmediate::toDouble(this); > > > it should be isNumber(this) no? Otherwise you'll call toDouble on > jsUndefined(), no? No, that's correct. All immediates prefer numeric conversion over string, and toDouble gives the right numeric equivalent. > Assuming you've run-javascriptcore-tests and fast/js (and you answer my > unsigned query), this is fine. I can mark it r+ when you reappear on IRC. I did run JavaScriptCore tests. About to run layout tests.
Comment on attachment 16871 [details] patch r=me assuming all the layout tests pass
This was landed in r27086.