[Webkit-unassigned] [Bug 20928] Speed up JS property enumeration by caching entire PropertyNameArray

[bugzilla-daemon at webkit.org]

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


darin at apple.com changed:

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




------- Comment #2 from darin at apple.com  2008-09-18 23:45 PDT -------
(From update of attachment 23556)
JSPropertyNameIterator::invalidate is so simple now it should probably be
inlined. Maybe also use clear() instead of assigning 0?

+    , m_data(0)

Don't need that for RefPtr.

+inline JSPropertyNameIterator::JSPropertyNameIterator(JSObject* object,
PropertyNameArrayData* propertyNameArrayData)

Should PropertyNameArrayData* should be PassRefPtr, since this takes ownership.

+        void setCachedPrototypeChain(StructureIDChain* cachedPrototypeChain) {
m_cachedPrototypeChain = cachedPrototypeChain; }

+        void setData(PropertyNameArrayData* data) { m_data = data; } 

Should be PassRefPtr since these take ownership.

+        if ((*a).get() != (*b).get())
+            return false;

These would read better with -> but can't you just omit the get() altogether?

+        if (!(*a).get())
+            return true;

This would read better with -> but can't you just omit the get() altogether?

+    ASSERT_NOT_REACHED();
+    return false;

This will just create a warning on Windows and make the build fail. Leave it
out.

+    // FIXME: This breaks the optimization for the JSGlobalObject. 

I think you should write a clearer comment. Does this really need a FIXME?

I'm going to say review- because of the number of actionable comments.


-- 
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.