Bug 51359 - Added PageAllocationAligned, a cross-platform abstraction for memory allocations with arbitrary alignment requirements
Summary: Added PageAllocationAligned, a cross-platform abstraction for memory allocati...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2010-12-20 15:55 PST by Geoffrey Garen
Modified: 2010-12-21 16:00 PST (History)
10 users (show)

See Also:


Attachments
Patch (42.43 KB, patch)
2010-12-20 16:50 PST, Geoffrey Garen
no flags Details | Formatted Diff | Diff
Patch (42.45 KB, patch)
2010-12-20 18:56 PST, Geoffrey Garen
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Cleaned up, fixed build errors, cleaned up WINCE extra decommit, re-added check for successful release. (52.63 KB, patch)
2010-12-21 15:20 PST, Gavin Barraclough
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2010-12-20 15:55:14 PST
Added PageAllocationAligned, a cross-platform abstraction for memory allocations with arbitrary alignment requirements
Comment 1 Geoffrey Garen 2010-12-20 16:50:52 PST
Created attachment 77055 [details]
Patch
Comment 2 Geoffrey Garen 2010-12-20 16:52:40 PST
<rdar://problem/8107952>
Comment 3 Gavin Barraclough 2010-12-20 17:06:27 PST
Comment on attachment 77055 [details]
Patch

might be nice to remove m_trueBase on DARWIN, it's not really needed.  your call, r+
Comment 4 Geoffrey Garen 2010-12-20 18:43:46 PST
I sat down to remove m_trueBase on DARWIN and realized that it would be more involved than just an #ifdef around the data member -- to satisfy unused parameter warnings, you'd either need to give DARWIN its own constructor, or you'd need to #ifdef out a portion of the shared constructor and #ifdef in an UNUSED_PARAM warning.

My gut says to stick with the simpler solution, since we're only talking about 4-8 bytes in an allocation that's >= 4K.
Comment 5 Geoffrey Garen 2010-12-20 18:56:18 PST
Created attachment 77063 [details]
Patch
Comment 6 Geoffrey Garen 2010-12-20 18:58:41 PST
Comment on attachment 77063 [details]
Patch

Here's an updated version of the patch with project file conflicts resolved.

Unfortunately, I'm not going to have time to land this before I leave for vacation. 

I'd sure appreciate it if a WebKit committer (human or robot!) could land this for me.
Comment 7 WebKit Commit Bot 2010-12-20 19:00:00 PST
Comment on attachment 77063 [details]
Patch

Rejecting attachment 77063 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'apply-attachment', '--non-interactive', 77063]" exit_code: 2
Last 500 characters of output:
ned.cpp
patching file JavaScriptCore/wtf/PageAllocationAligned.h
patching file JavaScriptCore/wtf/Platform.h
patching file JavaScriptCore/wtf/wtf.pri
patching file JavaScriptGlue/ChangeLog
patching file JavaScriptGlue/ForwardingHeaders/wtf/PageAllocationAligned.h
patching file WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file WebCore/ForwardingHeaders/wtf/PageAllocationAligned.h

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/7248073
Comment 8 WebKit Review Bot 2010-12-20 19:13:19 PST
Attachment 77063 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7295083
Comment 9 Early Warning System Bot 2010-12-20 19:21:37 PST
Attachment 77063 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7245069
Comment 10 WebKit Review Bot 2010-12-20 20:38:53 PST
Attachment 77063 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7263078
Comment 11 Build Bot 2010-12-20 21:43:25 PST
Attachment 77063 [details] did not build on win:
Build output: http://queues.webkit.org/results/7249084
Comment 12 Gavin Barraclough 2010-12-21 15:20:18 PST
Created attachment 77157 [details]
Cleaned up, fixed build errors, cleaned up WINCE extra decommit, re-added check for successful release.
Comment 13 Oliver Hunt 2010-12-21 15:36:26 PST
Comment on attachment 77157 [details]
Cleaned up, fixed build errors, cleaned up WINCE extra decommit, re-added check for successful release.

r=me
Comment 14 Gavin Barraclough 2010-12-21 15:55:21 PST
Oh, also, that patch fixed a bug in Geoff's last revision – deallocate() on non-DARWIN platforms was releasing 'size' bytes of memory, but be needed to release the amount we had reserved.

Fixed in r74431.
Comment 15 WebKit Review Bot 2010-12-21 16:00:41 PST
http://trac.webkit.org/changeset/74431 might have broken SnowLeopard Intel Release (Build)