JavaScript variable access currently looks up most variables twice. Related to this is the fact that hasOwnProperty and get effectively duplicate some functionality and need to be kept in sync. It would be nice to clean this up.
Created attachment 3077 [details] property change patch - changes get() to getOwnProperty()
+ if (!v || v == UndefinedImp::staticUndefined) + return false; Our code seems to disagree about whether to use "X.type() == UndefinedType", "X.isUndefined()", or "X== UndefinedImp::staticUndefined". Is there a Good Reason (TM) for this, or should we standardize it?
It happens that since the undefined value is unique, then so long as there is an interpreter in existence, all three will mean the same thing. However, comparing for == UndefinedImp::staticUndefined is likely faster than doing the type check, since it is a simple pointer compare rather than a virtual function call. So probably the best option would be to make isUndefined use the comparison to staticUndefined, and then use the isUndefined call everywhere for the sake of abstraction. In this case I used the "compare to staticUndefined" technique because it's what the code already did, and not for any real reason.
The patch I have sitting in my tree does exactly what Maciej suggests among many other optimizations. It changes all checks for Undefined to use isUndefined() and makes it use the fastest idiom (which seems to be comparing with the global singleton instance).
Comment on attachment 3077 [details] property change patch - changes get() to getOwnProperty() Since Value already has a distinct "null" value, why didn't you use that instead of adding a new result parameter? The reason I ask is that once I change Value to ValueImp, it because quite natural to have NULL mean "I don't have a property" even though it's in-band signalling. The change would be smaller if getOwnProperty simply returned Value() when there is no property, which is what getDirect does. I also think this would result in a patch with many fewer lines changed. Otherwise this patch looks great to me. By the way, my pending patch is going to get us a bunch *more* speedup on top of this, so I'm pretty excited about the combination.
I considered and rejected the in-band signalling option, because it wouldn't fit very well with the eventual plan to eliminate hasOwnProperty entirely. In that case you definitely need separate returns for presence of a property and its actual value. And this will be even more the case with the eventual property slot design. We can make access and simple assignment efficient without property slots but they are still needed to make update operations like "++" and "+=" efficient, and these are quite common both in benchmarks and in real-world web pages.
Comment on attachment 3077 [details] property change patch - changes get() to getOwnProperty() Maciej convinced me that as a first step, this use of bool is superior to the in-band signaling of using Value(), given what we plan next. I re-reviewed the entire patch. r=me
Maciej landed this today.