Bug 78610 - Factor out allocation in CopySpace into a separate CopyAllocator
Summary: Factor out allocation in CopySpace into a separate CopyAllocator
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Hahnenberg
URL:
Keywords:
Depends on: 78573
Blocks: 78612 79053
  Show dependency treegraph
 
Reported: 2012-02-14 09:38 PST by Mark Hahnenberg
Modified: 2012-02-20 16:33 PST (History)
2 users (show)

See Also:


Attachments
Patch (28.78 KB, patch)
2012-02-18 19:19 PST, Mark Hahnenberg
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Hahnenberg 2012-02-14 09:38:42 PST
Filip Pizlo, from bug 75181:

"Also, it would be good if:

...

- The data needed to allocate in the current block was kept in BumpSpace, or in something like a BumpAllocator class.  I.e. if you want to allocate stuff (so you're either a GC thread or you're the application) then you have an instance of BumpAllocator that is easily reachable (say by offsetting into JSGlobalData, if you're the application), and then all of the fields (i.e. m_offset) needed to allocate are inside of BumpAllocator.  Right now you're taking an extra load to get to the BumpBlock::m_offset.  The BumpBlock::m_offset field can then be updated after you switch from one block to another.

...

The point is that it will (a) simplify the inline asm code that the JIT will use for allocation and (b) speed things up a fair bit as we start relying on this allocator more."

Couldn't have said it better myself. This change will also make the overall design between MarkedSpace and CopySpace more consistent.
Comment 1 Mark Hahnenberg 2012-02-18 19:19:28 PST
Created attachment 127726 [details]
Patch
Comment 2 Mark Hahnenberg 2012-02-18 20:31:13 PST
I also moved a bunch of stuff from the headers for CopiedSpace to the cpp file to reduce code bloat/compile times.
Comment 3 Oliver Hunt 2012-02-20 15:17:10 PST
Comment on attachment 127726 [details]
Patch

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

You need to add the new files to the many a varied project files that exist, otherwise we may end up with build issues in future when they miss changes to dependencies.

Also add copyright to the new files.

r=me with those fixes, and after adding copyright info to the new files.

> Source/JavaScriptCore/heap/CopiedAllocator.h:1
> +#ifndef CopiedAllocator_h

You need a copyright header here
Comment 4 Mark Hahnenberg 2012-02-20 15:41:53 PST
Committed r108267: <http://trac.webkit.org/changeset/108267>