Bug 168754 - Remove the demand executable allocator
Summary: Remove the demand executable allocator
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Miller
URL:
Keywords:
: 154749 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-02-22 16:12 PST by Keith Miller
Modified: 2017-02-23 10:24 PST (History)
10 users (show)

See Also:


Attachments
Patch (45.32 KB, patch)
2017-02-22 16:20 PST, Keith Miller
saam: review+
Details | Formatted Diff | Diff
benchmark results (86.79 KB, text/plain)
2017-02-22 16:23 PST, Keith Miller
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Miller 2017-02-22 16:12:41 PST
Remove the demand executable allocator
Comment 1 Keith Miller 2017-02-22 16:20:25 PST
Created attachment 302459 [details]
Patch
Comment 2 WebKit Commit Bot 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.
Comment 3 Saam Barati 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
Comment 4 Keith Miller 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.
Comment 5 Keith Miller 2017-02-22 17:39:22 PST
Committed r212867: <http://trac.webkit.org/changeset/212867>
Comment 6 Csaba Osztrogonác 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.
Comment 7 Csaba Osztrogonác 2017-02-23 08:59:45 PST
*** Bug 154749 has been marked as a duplicate of this bug. ***
Comment 8 Keith Miller 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.
Comment 9 Keith Miller 2017-02-23 10:24:16 PST
cloop should, hopefully, be fixed in: http://trac.webkit.org/changeset/212900