[webkit-reviews] review denied: [Bug 8177] Javascript search incredibly slow : [Attachment 8882] proposed patch

[bugzilla-request-daemon at opendarwin.org]

Darin Adler <darin at apple.com> has denied Darin Adler <darin at apple.com>'s
request for review:
Bug 8177: Javascript search incredibly slow
http://bugzilla.opendarwin.org/show_bug.cgi?id=8177

Attachment 8882: proposed patch
http://bugzilla.opendarwin.org/attachment.cgi?id=8882&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
m_CollectionInfo should be m_collectionInfo (both of them) and
m_NameCollectionInfo should be m_nameCollectionInfo.

NUM_CACHEABLE_TYPES no longer seems to be a good name since now it's just the
dividing line between ones that have no name and ones that have a name. Also no
need for it to be all capitals -- macros are supposed to have all capital
names, but that's not appropriate for enums or constants (I know, preexisting
issue, not something you did).

+    HashMap<String, HTMLCollection::CollectionInfo>::iterator iter =
map.find(name);
+    if (iter != map.end())
+	 return &iter->second;
+    
+    return &map.add(name, HTMLCollection::CollectionInfo()).first->second;

I think it would be cool to just say:

    if (iter == map.end())
	iter = map.add(name, ...);

to collapse the two cases into one.

Is there goint to be trouble having lots of cached collections under different
names, with no limit on the number? Could this lead to memory bloat?

Otherwise looks great.

review- just for the nitpicks above.