Bug 192013 - Introducing a ENABLE_SEPARATED_WX_HEAP macro.
Summary: Introducing a ENABLE_SEPARATED_WX_HEAP macro.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-11-27 10:48 PST by Mark Lam
Modified: 2018-11-27 11:48 PST (History)
5 users (show)

See Also:

Attachments
proposed patch. (6.32 KB, patch)
2018-11-27 11:09 PST, Mark Lam 		keith_miller: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2018-11-27 10:48:16 PST 
This makes the code a little more readable.  See https://bugs.webkit.org/show_bug.cgi?id=190022#c3.

<rdar://problem/45494310>
Comment 1 Mark Lam 2018-11-27 11:09:08 PST 
Created attachment 355749 [details]
proposed patch.
Comment 2 Keith Miller 2018-11-27 11:26:25 PST 
Comment on attachment 355749 [details]
proposed patch.

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

r=me.

> Source/JavaScriptCore/config.h:41
> +#if !defined(ENABLE_SEPARATED_WX_HEAP)

Nit: why not #ifndef?

> Source/JavaScriptCore/config.h:42
> +#if (!ENABLE(FAST_JIT_PERMISSIONS) || !CPU(ARM64E)) && PLATFORM(IOS_FAMILY) && CPU(ARM64)

Nit: Can we make this !(ENABLE(FAST_JIT_PERMISSIONS) && CPU(ARM64E)) && PLATFORM(IOS_FAMILY) && CPU(ARM64)?

I prefer to pull the negations out since I find it easier to read the inside of the parens then negate.
Comment 3 Mark Lam 2018-11-27 11:43:52 PST 
Comment on attachment 355749 [details]
proposed patch.

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

Thanks for the review.

>> Source/JavaScriptCore/config.h:41
>> +#if !defined(ENABLE_SEPARATED_WX_HEAP)
> 
> Nit: why not #ifndef?

So that I can do #endif // !defined(ENABLE_SEPARATED_WX_HEAP) below.

>> Source/JavaScriptCore/config.h:42
>> +#if (!ENABLE(FAST_JIT_PERMISSIONS) || !CPU(ARM64E)) && PLATFORM(IOS_FAMILY) && CPU(ARM64)
> 
> Nit: Can we make this !(ENABLE(FAST_JIT_PERMISSIONS) && CPU(ARM64E)) && PLATFORM(IOS_FAMILY) && CPU(ARM64)?
> 
> I prefer to pull the negations out since I find it easier to read the inside of the parens then negate.

I prefer the original.  It reads better as the individual conditions for enabling the SEPARATED_WX_HEAP.  I'm going to keep this as is.
Comment 4 Mark Lam 2018-11-27 11:48:04 PST 
Landed in r238564: <http://trac.webkit.org/r238564>.