Bug 70364 – Switched ropes from malloc memory to GC memory

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Geoffrey Garen]

Switched ropes from malloc memory to GC memory

[Geoffrey Garen]

Created attachment 111507 [details]
Patch

[Gavin Barraclough]

Comment on attachment 111507 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=111507&action=review

Hmmm, I wonder whether we still need m_fiberCount, if a JSString is a rope, is m_value null? (to remove the test of m_fiberCount in isRope)
Still, that's a separate question really.

After this patch, will we ever be creating RopeImpls?
Should this patch not be removing RopeImpl.cpp/.h from svn?

r+, but I think you should probably be using jsNontrivialString in the case I point out, and probably remove the RopeImpl files.

> Source/JavaScriptCore/runtime/JSValue.cpp:222
> +        return JSString::create(exec->globalData(), UString("undefined").impl());

Maybe we should have an JSString::create(ExecState*, const char*) helper? - 
Actually, I think we have one! - this should call:

inline JSString* jsNontrivialString(JSGlobalData* globalData, const char* s)

> Source/JavaScriptCore/runtime/Operations.h:96
>      ALWAYS_INLINE JSValue jsString(ExecState* exec, JSValue thisValue)

This method is really poorly named (my bad, I believe :-( ) - there is nothing in the name indicating it does anything to do with arguments, we should really rename it to something more explicit.

> Source/JavaScriptCore/runtime/SmallStrings.cpp:109
> +    m_emptyString = JSString::createHasOtherOwner(*globalData, UString("").impl());

This could be 'StringImpl::empty()'

> Source/JavaScriptCore/runtime/StringPrototype.cpp:640
> +            : jsString(exec, asString(thisValue), JSString::create(exec->globalData(), v.toString(exec).impl())));

We create JSStrings with arguments like this in quite a few places, maybe we should have an JSString::create(ExecState*, JSValue)?

[Geoffrey Garen]

> > Source/JavaScriptCore/runtime/JSValue.cpp:222
> > +        return JSString::create(exec->globalData(), UString("undefined").impl());
> 
> Maybe we should have an JSString::create(ExecState*, const char*) helper? - 
> Actually, I think we have one! - this should call:
> 
> inline JSString* jsNontrivialString(JSGlobalData* globalData, const char* s)

Ah yes. Actually, all of my direct calls to JSString::create are broken on Windows, since they miss the glorious call to fixupVPtr. I'll fix those, and fix the rest in a follow-up patch.

[Geoffrey Garen]

Committed r97827: <http://trac.webkit.org/changeset/97827>