Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1913083002:
[heap] Uncommit pooled pages concurrently (Closed)
|
Created:
4 years, 8 months ago by Michael Lippautz Modified:
4 years, 7 months ago Reviewers:
Hannes Payer (out of office) CC:
Hannes Payer (out of office), ulan, v8-reviews_googlegroups.com Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[heap] Uncommit pooled pages concurrently
- Move the concurrent unmapping to MemoryAllocator
- Hide (private) members where possible
- MemoryAllocator:Free is now the bottleneck for freeing
- Pooled pages are either allocated from a set of pooled pages are obtained
through work stealing from the concurrent unmapper
BUG=chromium:605866, chromium:581412
LOG=N
Committed: https://crrev.com/2158df87116906160cebc3ad20c97f454822da03
Cr-Commit-Position: refs/heads/master@{#35797}
Patch Set 1 : #Patch Set 2 : Reworked to support stealing #Patch Set 3 : Fix stealing ;) #Patch Set 4 : Exclude executable pages from pooling #
Total comments: 8
Patch Set 5 : Addressed comments #
Messages
Total messages: 39 (22 generated)
Description was changed from ========== [heap] Uncommit pooled pages lazily - Move the concurrent unmapping to MemoryAllocator - Hide (private) members where possible - MemoryAllocator:Free is now the bottleneck for freeing - Allocate pooled pages to be freed concurrently BUG=chromium:605866 LOG=N ========== to ========== [heap] Uncommit pooled pages concurrently - Move the concurrent unmapping to MemoryAllocator - Hide (private) members where possible - MemoryAllocator:Free is now the bottleneck for freeing - Allocate pooled pages to be freed concurrently BUG=chromium:605866 LOG=N ==========
Description was changed from ========== [heap] Uncommit pooled pages concurrently - Move the concurrent unmapping to MemoryAllocator - Hide (private) members where possible - MemoryAllocator:Free is now the bottleneck for freeing - Allocate pooled pages to be freed concurrently BUG=chromium:605866 LOG=N ========== to ========== [heap] Uncommit pooled pages concurrently - Move the concurrent unmapping to MemoryAllocator - Hide (private) members where possible - MemoryAllocator:Free is now the bottleneck for freeing - Allocate pooled pages to be freed concurrently Addresses the regression for syscalls related to uncommitting on Android. BUG=chromium:605866 LOG=N ==========
Patchset #1 (id:1) has been deleted
Description was changed from ========== [heap] Uncommit pooled pages concurrently - Move the concurrent unmapping to MemoryAllocator - Hide (private) members where possible - MemoryAllocator:Free is now the bottleneck for freeing - Allocate pooled pages to be freed concurrently Addresses the regression for syscalls related to uncommitting on Android. BUG=chromium:605866 LOG=N ========== to ========== [heap] Uncommit pooled pages concurrently - Move the concurrent unmapping to MemoryAllocator - Hide (private) members where possible - MemoryAllocator:Free is now the bottleneck for freeing - Allocate pooled pages to be freed concurrently Addresses the regression for syscalls related to uncommitting on Android. BUG=chromium:605866, chromium:581412 LOG=N ==========
The CQ bit was checked by mlippautz@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1913083002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1913083002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_asan_rel_ng on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...) v8_linux64_asan_rel_ng_triggered on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng_trig...)
Patchset #1 (id:20001) has been deleted
The CQ bit was checked by mlippautz@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1913083002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1913083002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== [heap] Uncommit pooled pages concurrently - Move the concurrent unmapping to MemoryAllocator - Hide (private) members where possible - MemoryAllocator:Free is now the bottleneck for freeing - Allocate pooled pages to be freed concurrently Addresses the regression for syscalls related to uncommitting on Android. BUG=chromium:605866, chromium:581412 LOG=N ========== to ========== [heap] Uncommit pooled pages concurrently - Move the concurrent unmapping to MemoryAllocator - Hide (private) members where possible - MemoryAllocator:Free is now the bottleneck for freeing - Pooled pages are either allocated from a set of pooled pages are obtained through work stealing from the concurrent unmapper. Addresses the regression for syscalls related to uncommitting on Android. BUG=chromium:605866, chromium:581412 LOG=N ==========
Description was changed from ========== [heap] Uncommit pooled pages concurrently - Move the concurrent unmapping to MemoryAllocator - Hide (private) members where possible - MemoryAllocator:Free is now the bottleneck for freeing - Pooled pages are either allocated from a set of pooled pages are obtained through work stealing from the concurrent unmapper. Addresses the regression for syscalls related to uncommitting on Android. BUG=chromium:605866, chromium:581412 LOG=N ========== to ========== [heap] Uncommit pooled pages concurrently - Move the concurrent unmapping to MemoryAllocator - Hide (private) members where possible - MemoryAllocator:Free is now the bottleneck for freeing - Pooled pages are either allocated from a set of pooled pages are obtained through work stealing from the concurrent unmapper. Addresses the regression for syscalls related to uncommitting on Android Nexus 5X. BUG=chromium:605866, chromium:581412 LOG=N ==========
The CQ bit was checked by mlippautz@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1913083002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1913083002/60001
Description was changed from ========== [heap] Uncommit pooled pages concurrently - Move the concurrent unmapping to MemoryAllocator - Hide (private) members where possible - MemoryAllocator:Free is now the bottleneck for freeing - Pooled pages are either allocated from a set of pooled pages are obtained through work stealing from the concurrent unmapper. Addresses the regression for syscalls related to uncommitting on Android Nexus 5X. BUG=chromium:605866, chromium:581412 LOG=N ========== to ========== [heap] Uncommit pooled pages concurrently - Move the concurrent unmapping to MemoryAllocator - Hide (private) members where possible - MemoryAllocator:Free is now the bottleneck for freeing - Pooled pages are either allocated from a set of pooled pages are obtained through work stealing from the concurrent unmapper BUG=chromium:605866, chromium:581412 LOG=N ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_asan_rel_ng on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...) v8_linux64_asan_rel_ng_triggered on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng_trig...)
The CQ bit was checked by mlippautz@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1913083002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1913083002/100001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
mlippautz@chromium.org changed reviewers: + hpayer@chromium.org
PTAL
lgtm https://codereview.chromium.org/1913083002/diff/100001/src/heap/spaces.cc File src/heap/spaces.cc (right): https://codereview.chromium.org/1913083002/diff/100001/src/heap/spaces.cc#new... src/heap/spaces.cc:394: // Large chunks. This is not just large junks but also code. Can we do that before the regular chunks. Micro-optimizations. https://codereview.chromium.org/1913083002/diff/100001/src/heap/spaces.h File src/heap/spaces.h (right): https://codereview.chromium.org/1913083002/diff/100001/src/heap/spaces.h#newc... src/heap/spaces.h:1309: kRegularProcess, // Pages of kPageSize that do not live in a CodeRange kRegular https://codereview.chromium.org/1913083002/diff/100001/src/heap/spaces.h#newc... src/heap/spaces.h:1311: kNonRegularProcess, // Large chunks and executable chunks. kNonRegular https://codereview.chromium.org/1913083002/diff/100001/src/heap/spaces.h#newc... src/heap/spaces.h:1312: kPooledOutput, // Pooled chunks, already uncommited and ready for kPooled
https://codereview.chromium.org/1913083002/diff/100001/src/heap/spaces.cc File src/heap/spaces.cc (right): https://codereview.chromium.org/1913083002/diff/100001/src/heap/spaces.cc#new... src/heap/spaces.cc:394: // Large chunks. On 2016/04/26 13:01:27, Hannes Payer wrote: > This is not just large junks but also code. > > Can we do that before the regular chunks. Micro-optimizations. As discussed offline: Let's keep the order. Fixed the comment though. https://codereview.chromium.org/1913083002/diff/100001/src/heap/spaces.h File src/heap/spaces.h (right): https://codereview.chromium.org/1913083002/diff/100001/src/heap/spaces.h#newc... src/heap/spaces.h:1309: kRegularProcess, // Pages of kPageSize that do not live in a CodeRange On 2016/04/26 13:01:27, Hannes Payer wrote: > kRegular Done. https://codereview.chromium.org/1913083002/diff/100001/src/heap/spaces.h#newc... src/heap/spaces.h:1311: kNonRegularProcess, // Large chunks and executable chunks. On 2016/04/26 13:01:27, Hannes Payer wrote: > kNonRegular Done. https://codereview.chromium.org/1913083002/diff/100001/src/heap/spaces.h#newc... src/heap/spaces.h:1312: kPooledOutput, // Pooled chunks, already uncommited and ready for On 2016/04/26 13:01:27, Hannes Payer wrote: > kPooled Done.
The CQ bit was checked by mlippautz@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1913083002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1913083002/120001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by mlippautz@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from hpayer@chromium.org Link to the patchset: https://codereview.chromium.org/1913083002/#ps120001 (title: "Addressed comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1913083002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1913083002/120001
Message was sent while issue was closed.
Committed patchset #5 (id:120001)
Message was sent while issue was closed.
Description was changed from ========== [heap] Uncommit pooled pages concurrently - Move the concurrent unmapping to MemoryAllocator - Hide (private) members where possible - MemoryAllocator:Free is now the bottleneck for freeing - Pooled pages are either allocated from a set of pooled pages are obtained through work stealing from the concurrent unmapper BUG=chromium:605866, chromium:581412 LOG=N ========== to ========== [heap] Uncommit pooled pages concurrently - Move the concurrent unmapping to MemoryAllocator - Hide (private) members where possible - MemoryAllocator:Free is now the bottleneck for freeing - Pooled pages are either allocated from a set of pooled pages are obtained through work stealing from the concurrent unmapper BUG=chromium:605866, chromium:581412 LOG=N Committed: https://crrev.com/2158df87116906160cebc3ad20c97f454822da03 Cr-Commit-Position: refs/heads/master@{#35797} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/2158df87116906160cebc3ad20c97f454822da03 Cr-Commit-Position: refs/heads/master@{#35797}
Message was sent while issue was closed.
A revert of this CL (patchset #5 id:120001) has been created in https://codereview.chromium.org/1925563003/ by hablich@chromium.org. The reason for reverting is: Creates a spike of OOM(v8) crashers on Win32.. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
