Bug 26750 - Allow custom memory allocation control for JavaScriptCore/runtime directory's classes
Summary: Allow custom memory allocation control for JavaScriptCore/runtime directory's...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-26 01:38 PDT by Zoltan Horvath
Modified: 2009-08-07 12:52 PDT (History)
2 users (show)

See Also:


Attachments
proposed patch (4.36 KB, patch)
2009-06-26 01:40 PDT, Zoltan Horvath
no flags Details | Formatted Diff | Diff
updated proposed patch (4.43 KB, patch)
2009-06-26 01:41 PDT, Zoltan Horvath
eric: review-
Details | Formatted Diff | Diff
updated proposed patch (1.25 KB, patch)
2009-07-28 00:45 PDT, Zoltan Horvath
eric: commit-queue+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zoltan Horvath 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
Comment 1 Zoltan Horvath 2009-06-26 01:40:02 PDT
Created attachment 31916 [details]
proposed patch
Comment 2 Zoltan Horvath 2009-06-26 01:41:09 PDT
Created attachment 31917 [details]
updated proposed patch
Comment 3 Eric Seidel (no email) 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.
Comment 4 Eric Seidel (no email) 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
Comment 5 Eric Seidel (no email) 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])
Comment 6 Eric Seidel (no email) 2009-06-26 10:30:36 PDT
Comment on attachment 31917 [details]
updated proposed patch

Caused crash on the bots.
Comment 7 Eric Seidel (no email) 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
Comment 8 Eric Seidel (no email) 2009-06-26 10:37:23 PDT
I need to add a --no-close option to my landing script.
Comment 9 Zoltan Horvath 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.
Comment 10 Darin Adler 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!
Comment 11 Zoltan Horvath 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.
Comment 12 Zoltan Horvath 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.
Comment 13 Eric Seidel (no email) 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?
Comment 14 Adam Barth 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
Comment 15 Adam Barth 2009-08-07 12:52:15 PDT
All reviewed patches have been landed.  Closing bug.