Bug 22609 – Provide a build-time choice when generating hash tables for properties of built-in DOM objects

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[David Kilzer (:ddkilzer)]

*SUMMARY
WebKit r31147 <http://trac.webkit.org/changeset/31147> replaced a "compact" hash with a perfect hash for storing properties of built-in DOM objects.  Unfortunately, this tends to use more memory to store rather small amounts of data.  It would be nice to restore the original "compact" hash code and to provide a build-time choice to select the hashing algorithm.

* NOTES
<rdar://problem/6331749>

[David Kilzer (:ddkilzer)]

Created attachment 25694 [details]
Patch v1

Passes all JavaScriptCore tests and Layout Tests with ENABLE_PERFECT_HASH_SIZE set to 0 in JavaScriptCore/runtime/Lookup.h.

[Darin Adler]

Comment on attachment 25694 [details]
Patch v1

> +#if ENABLE(PERFECT_HASH_SIZE)
>  void HashTable::createTable(JSGlobalData* globalData) const

Since both of these implementations share the same function name I think it would be more elegant to put the #if around the body of the function rather than the entire function.

> +// Define ENABLE_PERFECT_HASH_SIZE to 0 to save memory
> +#define ENABLE_PERFECT_HASH_SIZE 1

This is not a great comment. Why would anyone let the value be 1 if setting it to 0 saves memory?

r=me

[David Kilzer (:ddkilzer)]

(In reply to comment #2)
> (From update of attachment 25694 [details] [review])
> > +#if ENABLE(PERFECT_HASH_SIZE)
> >  void HashTable::createTable(JSGlobalData* globalData) const
> 
> Since both of these implementations share the same function name I think it
> would be more elegant to put the #if around the body of the function rather
> than the entire function.

Will do.

> > +// Define ENABLE_PERFECT_HASH_SIZE to 0 to save memory
> > +#define ENABLE_PERFECT_HASH_SIZE 1
> 
> This is not a great comment. Why would anyone let the value be 1 if setting it
> to 0 saves memory?

I'll describe the speed/memory trade-off instead when landing.

[David Kilzer (:ddkilzer)]

(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 25694 [details] [review] [review])
> > > +#if ENABLE(PERFECT_HASH_SIZE)
> > >  void HashTable::createTable(JSGlobalData* globalData) const
> > 
> > Since both of these implementations share the same function name I think it
> > would be more elegant to put the #if around the body of the function rather
> > than the entire function.
> 
> Will do.

Will also make this change to this method in Lookup.h:

+         ALWAYS_INLINE const HashEntry* entry(const Identifier& identifier) const

[David Kilzer (:ddkilzer)]

$ git svn dcommit
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	JavaScriptCore/ChangeLog
	M	JavaScriptCore/create_hash_table
	M	JavaScriptCore/runtime/Lookup.cpp
	M	JavaScriptCore/runtime/Lookup.h
	M	JavaScriptCore/runtime/Structure.cpp
	M	WebCore/ChangeLog
	M	WebCore/bindings/scripts/CodeGeneratorJS.pm
Committed r39056

http://trac.webkit.org/changeset/39056

[David Kilzer (:ddkilzer)]

Follow-up ChangeLog entry fix:

$ git svn dcommit
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	JavaScriptCore/ChangeLog
	M	WebCore/ChangeLog
Committed r39057

http://trac.webkit.org/changeset/39057