Summary: | bmalloc: support physical page sizes that don't match the virtual page size (take 2) | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Geoffrey Garen <ggaren> | ||||||
Component: | bmalloc | Assignee: | Geoffrey Garen <ggaren> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | barraclough, buildbot, ggaren, kling, msaboff, rniwa | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Geoffrey Garen
2016-03-29 19:50:06 PDT
Created attachment 275168 [details]
Patch
Comment on attachment 275168 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=275168&action=review r=me > Source/bmalloc/bmalloc/VMAllocate.h:55 > + static size_t cached; > + if (!cached) > + cached = sysconf(_SC_PAGESIZE); Is this meaningfully different from static size_t cached = sysconf(_SC_PAGESIZE); ? > Source/bmalloc/bmalloc/VMAllocate.h:64 > + static size_t cached; > + if (!cached) > + cached = log2(vmPageSize()); > + return cached; Same question. > Source/bmalloc/bmalloc/VMAllocate.h:96 > + static size_t cached; > + if (!cached) > + cached = sysconf(_SC_PAGESIZE); > + return cached; Here too. Comment on attachment 275168 [details] Patch Attachment 275168 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1066936 New failing tests: inspector/debugger/breakpoint-condition-with-exception.html inspector/debugger/breakpoint-eval-with-exception.html Created attachment 275174 [details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
> > + static size_t cached;
> > + if (!cached)
> > + cached = sysconf(_SC_PAGESIZE);
>
> Is this meaningfully different from
> static size_t cached = sysconf(_SC_PAGESIZE);
static will use a separate variable to indicate whether the value has been initialized. So, this is one load fewer. Meh.
(In reply to comment #5) > > > + static size_t cached; > > > + if (!cached) > > > + cached = sysconf(_SC_PAGESIZE); > > > > Is this meaningfully different from > > static size_t cached = sysconf(_SC_PAGESIZE); > > static will use a separate variable to indicate whether the value has been > initialized. So, this is one load fewer. Meh. Better answer: It is not thread-safe to use a separate control variable to indicate that cached has been set. The control variable can appear to be set before cached appears to be set. mac-debug-ewe failure appears to be a so-called "flaky" test. Windows build failure is a pre-existing issue: WebCore/CoreGraphicsSPI.h. mac-wk2-ews failure -- I'm not sure yet. Committed r198829: <http://trac.webkit.org/changeset/198829> |