[Webkit-unassigned] [Bug 20589] PropertyMap speedup

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=20589


ggaren at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #23252|review?                     |review-
               Flag|                            |




------- Comment #23 from ggaren at apple.com  2008-09-08 16:34 PDT -------
(From update of attachment 23252)
I prefer the 1 item cache because it's a bigger win on SunSpider, and it's
simpler.

I think the best way to solve the JSOBJECT_READONLY problem is simply to move
the definition of property attributes into PropertyMap.h.

+#if DUMP_PROPERTYMAP_STATS
+        static int numLookupHits;
+#endif

This change is invalid, since it will increase the size of a JavaScript object,
which has a fixed size limit. I'm surprised that JavaScriptCore still builds
with this change applied -- we have compile-time ASSERTs that should cause a
build failure.

I think the best solution is just to leave DUMP_PROPERTYMAP_STATS out of the
patch. Alternatively, you could use a static counter in PropertyMap.cpp for
these stats. (You've already done that for numLookupMiss.) That wouldn't be
thread-safe, but that's OK, since it's just a debugging tool. I would also
rename this #define to DUMP_PROPERTYMAP_CACHE_HITS, and move it to
PropertyMap.cpp.

I don't like the change to make a bunch of get and put wrapper functions around
getInternal and putInternal functions. I think it bloats PropertyMap
substantially. A better solution would be just to write the caching logic
directly into all the existing get and put functions. 

I think the name "putLookup" is far too generic for a function in a class that
already uses the word "put" to mean something else. "recordLookup" and
"clearRecordedLookup" would be better names.

"keyLookup1" is a pretty generic name too. How about "lastLookup"?


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.