Bug 162514 - [JSC] Allow fixedExecutableMemoryPoolSize to be set during build
Summary: [JSC] Allow fixedExecutableMemoryPoolSize to be set during build
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Don Olmstead
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-23 14:49 PDT by Don Olmstead
Modified: 2016-09-26 11:12 PDT (History)
6 users (show)

See Also:


Attachments
Add macro (1.24 KB, patch)
2016-09-23 15:57 PDT, Don Olmstead
mark.lam: review+
Details | Formatted Diff | Diff
Fix nits (1.24 KB, patch)
2016-09-23 16:33 PDT, Don Olmstead
mark.lam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Don Olmstead 2016-09-23 14:49:30 PDT
Currently the memory pool size is set based on the CPU. The x64 version uses a 1GB pool size by default which is very large value. This value should be able to be specified by the port to allow overriding of the defaults.
Comment 1 Don Olmstead 2016-09-23 15:57:53 PDT
Created attachment 289715 [details]
Add macro

This allows the size in MB to be specified by the port. In CMake you would use add_definitions(-DFIXED_EXECUTABLE_MEMORY_POOL_SIZE_IN_MB=<value>) within the Platform cmake file within JavaScriptCore.

I'm open to changes to the name and also not sure if there should be any sort of static assert that ensures the validity of the value.
Comment 2 Mark Lam 2016-09-23 16:04:32 PDT
Comment on attachment 289715 [details]
Add macro

LGTM.  Did you want this reviewed?  If so, please set the patch r attributes to r?.  If you don't have committer privileges and want the patch committed, please set the patch cq attribute to cq?.
Comment 3 Mark Lam 2016-09-23 16:10:09 PDT
Comment on attachment 289715 [details]
Add macro

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

r=me with suggestion.

> Source/JavaScriptCore/jit/ExecutableAllocator.h:75
> +#if defined FIXED_EXECUTABLE_MEMORY_POOL_SIZE_IN_MB

nit: I prefer #ifdef FIXED_EXECUTABLE_MEMORY_POOL_SIZE_IN_MB or # if defined(FIXED_EXECUTABLE_MEMORY_POOL_SIZE_IN_MB)
Comment 4 Don Olmstead 2016-09-23 16:33:52 PDT
Created attachment 289719 [details]
Fix nits

Fixing nits
Comment 5 Konstantin Tokarev 2016-09-26 10:57:10 PDT
Comment on attachment 289719 [details]
Fix nits

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

> Source/JavaScriptCore/jit/ExecutableAllocator.h:75
> +#ifdef FIXED_EXECUTABLE_MEMORY_POOL_SIZE_IN_MB

I'd preferred #if defined(FIXED_EXECUTABLE_MEMORY_POOL_SIZE_IN_MB), it would be more in line with following #elif
Comment 6 Konstantin Tokarev 2016-09-26 11:12:12 PDT
Committed r206379: <http://trac.webkit.org/changeset/206379>