RESOLVED FIXED26750
Allow custom memory allocation control for JavaScriptCore/runtime directory's classes
https://bugs.webkit.org/show_bug.cgi?id=26750
Summary Allow custom memory allocation control for JavaScriptCore/runtime directory's...
Zoltan Horvath
Reported 2009-06-26 01:38:00 PDT
Inherits runtime directory's classes from FastAllocBase (bug #20422), which has been instantiated by 'new': class IdentifierTable -> instantiated in runtime/Identifier.cpp: line 70 stuct RegExpObjectData -> runtime/RegExpObject.cpp: line 62 class HashEntry -> runtime/Lookup.h: line 47 struct DateInstance::Cache -> runtime/DateInstance.cpp: line 33 class UString -> runtime/UString.cpp: line 212 struct RegExpConstructorPrivate -> runtime/RegExpConstructor.cpp: line 108 class ScopeChain -> untime/JSFunction.h: line 109
Attachments
proposed patch (4.36 KB, patch)
2009-06-26 01:40 PDT, Zoltan Horvath
no flags
updated proposed patch (4.43 KB, patch)
2009-06-26 01:41 PDT, Zoltan Horvath
eric: review-
updated proposed patch (1.25 KB, patch)
2009-07-28 00:45 PDT, Zoltan Horvath
eric: commit-queue+
Zoltan Horvath
Comment 1 2009-06-26 01:40:02 PDT
Created attachment 31916 [details] proposed patch
Zoltan Horvath
Comment 2 2009-06-26 01:41:09 PDT
Created attachment 31917 [details] updated proposed patch
Eric Seidel (no email)
Comment 3 2009-06-26 03:20:47 PDT
Comment on attachment 31917 [details] updated proposed patch Looks fine. I still think we could just land these all as one big patch.
Eric Seidel (no email)
Comment 4 2009-06-26 03:27:26 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ... M JavaScriptCore/ChangeLog M JavaScriptCore/runtime/DateInstance.cpp M JavaScriptCore/runtime/Identifier.cpp M JavaScriptCore/runtime/Lookup.h M JavaScriptCore/runtime/RegExpConstructor.cpp M JavaScriptCore/runtime/RegExpObject.h M JavaScriptCore/runtime/ScopeChain.h M JavaScriptCore/runtime/UString.h Committed r45259 http://trac.webkit.org/changeset/45259
Eric Seidel (no email)
Comment 5 2009-06-26 10:30:17 PDT
Rolling out http://trac.webkit.org/changeset/45259. Caused: http://build.webkit.org/builders/Leopard%20Intel%20Debug%20%28Tests%29/builds/2322 http://build.webkit.org/results/Leopard%20Intel%20Debug%20(Tests)/r45259%20(2322)/results.html plugins/undefined-property-crash.html ASSERTION FAILED: s <= HeapConstants<heapType>::cellSize (/Volumes/Big/WebKit-BuildSlave/leopard-intel-debug-tests/build/JavaScriptCore/runtime/Collector.cpp:278 void* JSC::Heap::heapAllocate(size_t) [with JSC::HeapType heapType = PrimaryHeap])
Eric Seidel (no email)
Comment 6 2009-06-26 10:30:36 PDT
Comment on attachment 31917 [details] updated proposed patch Caused crash on the bots.
Eric Seidel (no email)
Comment 7 2009-06-26 10:36:46 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ... M JavaScriptCore/ChangeLog M JavaScriptCore/runtime/DateInstance.cpp M JavaScriptCore/runtime/Identifier.cpp M JavaScriptCore/runtime/Lookup.h M JavaScriptCore/runtime/RegExpConstructor.cpp M JavaScriptCore/runtime/RegExpObject.h M JavaScriptCore/runtime/ScopeChain.h M JavaScriptCore/runtime/UString.h Committed r45269 http://trac.webkit.org/changeset/45269
Eric Seidel (no email)
Comment 8 2009-06-26 10:37:23 PDT
I need to add a --no-close option to my landing script.
Zoltan Horvath
Comment 9 2009-07-28 00:45:36 PDT
Created attachment 33606 [details] updated proposed patch I need to send one patch for one class, so we will see which causes the assertion.
Darin Adler
Comment 10 2009-07-28 10:13:18 PDT
I'm concerned, because this seems to indicate that adding FastAllocBase as a base class is making some objects bigger!
Zoltan Horvath
Comment 11 2009-07-28 10:51:07 PDT
I think I made a mistake in the r-d patch and inherit a class which has been instantiated by JS's GC also and that caused the problem.
Zoltan Horvath
Comment 12 2009-07-30 05:06:16 PDT
I've checked the problem: I've inherited DateInstance::Cache form FastAllocBase but DataInstance is a JSObject so this caused the problem. DataInstance::Cache doesn't need to be inherited.
Eric Seidel (no email)
Comment 13 2009-08-07 12:31:45 PDT
Comment on attachment 33606 [details] updated proposed patch Assuming everything still builds. Does this need to be public FastAllocBase?
Adam Barth
Comment 14 2009-08-07 12:52:10 PDT
Comment on attachment 33606 [details] updated proposed patch Clearing review flag on attachment: 33606 Committing to http://svn.webkit.org/repository/webkit/trunk ... M JavaScriptCore/ChangeLog M JavaScriptCore/runtime/RegExpObject.h Committed r46907 M JavaScriptCore/runtime/RegExpObject.h M JavaScriptCore/ChangeLog r46907 = 44404206ca651e03bd169e7e01bca730b59bcd41 (trunk) No changes between current HEAD and refs/remotes/trunk Resetting to the latest refs/remotes/trunk http://trac.webkit.org/changeset/46907
Adam Barth
Comment 15 2009-08-07 12:52:15 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.