[webkit-reviews] review granted: [Bug 55423] Clean up property tables in Structure : [Attachment 84145] The patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Gavin Barraclough
<barraclough at apple.com>'s request for review:
Bug 55423: Clean up property tables in Structure
https://bugs.webkit.org/show_bug.cgi?id=55423

Attachment 84145: The patch
https://bugs.webkit.org/attachment.cgi?id=84145&action=review

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=84145&action=review

Please consider moving implementations to inline functions out of the class
definition and moving the power of 2 math to StdLibExtras.h in a follow up
patch.

> Source/JavaScriptCore/runtime/PropertyMapHashTable.h:177
> +

An assertion that initialCapacity is >= other.size would be good.

> Source/JavaScriptCore/runtime/PropertyMapHashTable.h:373
> +    // we knoe capacity to be available.

Typo. Knoe!

> Source/JavaScriptCore/runtime/PropertyMapHashTable.h:467
> +    Vector<unsigned>* m_deletedOffsets;

Can this be an OwnPtr?

>> Source/JavaScriptCore/runtime/Structure.cpp:67
>> +//static const int smallMapThreshold = 1024;
> 
> Should have a space between // and comment  [whitespace/comments] [4]

Please delete.

>> Source/JavaScriptCore/runtime/Structure.cpp:71
>> +//static const unsigned tinyMapThreshold = 20;
> 
> Should have a space between // and comment  [whitespace/comments] [4]

Please delete.