[webkit-reviews] review denied: [Bug 86024] Enh: Hash Const JSString in Backing Stores to Save Memory : [Attachment 150492] Updated patch with changes suggested by reviewer
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jul 2 16:09:53 PDT 2012
Oliver Hunt <oliver at apple.com> has denied Michael Saboff <msaboff at apple.com>'s
request for review:
Bug 86024: Enh: Hash Const JSString in Backing Stores to Save Memory
https://bugs.webkit.org/show_bug.cgi?id=86024
Attachment 150492: Updated patch with changes suggested by reviewer
https://bugs.webkit.org/attachment.cgi?id=150492&action=review
------- Additional Comments from Oliver Hunt <oliver at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=150492&action=review
> Source/JavaScriptCore/ChangeLog:68
> -
> - * runtime/JSArray.cpp:
> + * runtime/JSArray.cpp:
?
> Source/JavaScriptCore/runtime/JSString.h:169
> + static JS_EXPORTDATA unsigned s_newStringsSinceLastGC;
This isn't thread safe -- we have multiple heaps independtly being GC'd so this
needs to be a per-heap field.
> Source/JavaScriptCore/runtime/JSString.h:195
> + static const unsigned s_hashConstLock = 1u << 2;
> + static const unsigned s_isHashConstSingleton = 1u << 1;
> + static const unsigned s_is8Bit = 1u;
Would be nicer (and less likely for the compiler to do anything stupid) as:
enum {
hashConstLock = 1u << 2,
isHashConstSingleton = 1u << 1,
s_is8Bit = 1u
};
More information about the webkit-reviews
mailing list