Bug 168754

Summary: Remove the demand executable allocator
Product: WebKit Reporter: Keith Miller <keith_miller>
Component: New BugsAssignee: Keith Miller <keith_miller>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cdumez, cmarcelo, commit-queue, dbates, mark.lam, msaboff, oliver, ossy, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
saam: review+
benchmark results none

Keith Miller
Reported 2017-02-22 16:12:41 PST
Remove the demand executable allocator
Attachments
Patch (45.32 KB, patch)
2017-02-22 16:20 PST, Keith Miller 		saam: review+
DetailsFormatted DiffDiff
benchmark results (86.79 KB, text/plain)
2017-02-22 16:23 PST, Keith Miller 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Keith Miller
Comment 1 2017-02-22 16:20:25 PST
Created attachment 302459 [details] Patch
WebKit Commit Bot
Comment 2 2017-02-22 16:22:15 PST
Attachment 302459 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/jit/ExecutableAllocator.cpp:63: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/jit/ExecutableAllocator.cpp:63: target_task is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/jit/ExecutableAllocator.cpp:64: Declaration has space between type name and * in mach_vm_address_t *target_address [whitespace/declaration] [3] ERROR: Source/JavaScriptCore/jit/ExecutableAllocator.cpp:64: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/jit/ExecutableAllocator.cpp:65: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/jit/ExecutableAllocator.cpp:66: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/jit/ExecutableAllocator.cpp:67: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/jit/ExecutableAllocator.cpp:68: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/jit/ExecutableAllocator.cpp:68: src_task is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/jit/ExecutableAllocator.cpp:69: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/jit/ExecutableAllocator.cpp:69: src_address is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/jit/ExecutableAllocator.cpp:70: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/jit/ExecutableAllocator.cpp:71: Declaration has space between type name and * in vm_prot_t *cur_protection [whitespace/declaration] [3] ERROR: Source/JavaScriptCore/jit/ExecutableAllocator.cpp:71: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/jit/ExecutableAllocator.cpp:72: Declaration has space between type name and * in vm_prot_t *max_protection [whitespace/declaration] [3] ERROR: Source/JavaScriptCore/jit/ExecutableAllocator.cpp:72: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/jit/ExecutableAllocator.cpp:73: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/jit/ExecutableAllocator.cpp:74: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 18 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Saam Barati
Comment 3 2017-02-22 16:22:56 PST
Comment on attachment 302459 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=302459&action=review > Source/JavaScriptCore/jit/ExecutableAllocator.cpp:2 > + * Copyright (C) 2008-2009, 2015 Apple Inc. All rights reserved. should be 2017
Keith Miller
Comment 4 2017-02-22 16:23:53 PST
Created attachment 302461 [details] benchmark results Benchmark results show a 15-20% regression on crypto-md5 on 32-bit MBP but I couldn't find any reason why such a regression should occur. I'm not sure that one 32-bit benchmark is worth the code debt.
Keith Miller
Comment 5 2017-02-22 17:39:22 PST
Committed r212867: <http://trac.webkit.org/changeset/212867>
Csaba Osztrogonác
Comment 6 2017-02-22 21:58:06 PST
(In reply to comment #5) > Committed r212867: <http://trac.webkit.org/changeset/212867> It broke the cloop build.
Csaba Osztrogonác
Comment 7 2017-02-23 08:59:45 PST
*** Bug 154749 has been marked as a duplicate of this bug. ***
Keith Miller
Comment 8 2017-02-23 10:18:04 PST
(In reply to comment #6) > (In reply to comment #5) > > Committed r212867: <http://trac.webkit.org/changeset/212867> > > It broke the cloop build. Fixing.
Keith Miller
Comment 9 2017-02-23 10:24:16 PST
cloop should, hopefully, be fixed in: http://trac.webkit.org/changeset/212900
Note You need to log in before you can comment on or make changes to this bug.