Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2548213004:
[heap] Use callbacks to dispatch store buffer operations. (Closed)
|
Created:
4 years ago by Hannes Payer (out of office) Modified:
3 years, 11 months ago CC:
v8-reviews_googlegroups.com, Hannes Payer (out of office), ulan Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[heap] Use callbacks to dispatch store buffer operations.
BUG=chromium:648568, chromium:669920
Committed: https://crrev.com/9c191a0cda081250d40586e3dbccf9209727d233
Cr-Commit-Position: refs/heads/master@{#41592}
Patch Set 1 #Patch Set 2 : format #Patch Set 3 : callbacks #Patch Set 4 : missing #Patch Set 5 : restructure #
Total comments: 2
Messages
Total messages: 37 (29 generated)
The CQ bit was checked by hpayer@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...) v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/17290) v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/13687)
The CQ bit was checked by hpayer@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...) v8_linux64_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)
The CQ bit was checked by hpayer@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [heap] Always add to store buffer. BUG= ========== to ========== [heap] Use callbacks to dispatch store buffer operations. BUG= ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...) v8_win_nosnap_shared_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)
The CQ bit was checked by hpayer@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...) v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/17548) v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/17459) v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/19191)
The CQ bit was checked by hpayer@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
hpayer@chromium.org changed reviewers: + mlippautz@chromium.org
hpayer@chromium.org changed reviewers: + ulan@chromium.org
lgtm
Description was changed from ========== [heap] Use callbacks to dispatch store buffer operations. BUG= ========== to ========== [heap] Use callbacks to dispatch store buffer operations. BUG=chromium:648568, chromium:669920 ==========
The CQ bit was checked by hpayer@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1481206619812330,
"parent_rev": "052d4f9b64873ce412cfd54f76a3f6c8ad742f41", "commit_rev":
"37f7f06a7d4c6e2cd8bebc8b4684fdffda510687"}
Message was sent while issue was closed.
Description was changed from ========== [heap] Use callbacks to dispatch store buffer operations. BUG=chromium:648568, chromium:669920 ========== to ========== [heap] Use callbacks to dispatch store buffer operations. BUG=chromium:648568, chromium:669920 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== [heap] Use callbacks to dispatch store buffer operations. BUG=chromium:648568, chromium:669920 ========== to ========== [heap] Use callbacks to dispatch store buffer operations. BUG=chromium:648568, chromium:669920 Committed: https://crrev.com/9c191a0cda081250d40586e3dbccf9209727d233 Cr-Commit-Position: refs/heads/master@{#41592} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/9c191a0cda081250d40586e3dbccf9209727d233 Cr-Commit-Position: refs/heads/master@{#41592}
Message was sent while issue was closed.
predrag.rudic@imgtec.com changed reviewers: + predrag.rudic@imgtec.com
Message was sent while issue was closed.
https://codereview.chromium.org/2548213004/diff/80001/src/heap/store-buffer.h File src/heap/store-buffer.h (right): https://codereview.chromium.org/2548213004/diff/80001/src/heap/store-buffer.h... src/heap/store-buffer.h:204: // store buffer operation. There is a problem with this code because it fails to compile on MIPS architecture for Android. It is found that this is due to bug in compiler. There is a workaround for this. If only types of deletion_callback and insertion_callback is replaced with function pointer, code compiles fine. Is it OK to upload patch that will fix this issue?
Message was sent while issue was closed.
https://codereview.chromium.org/2548213004/diff/80001/src/heap/store-buffer.h File src/heap/store-buffer.h (right): https://codereview.chromium.org/2548213004/diff/80001/src/heap/store-buffer.h... src/heap/store-buffer.h:204: // store buffer operation. On 2016/12/23 11:55:17, predrag.rudic wrote: > There is a problem with this code because it fails to compile on MIPS > architecture for Android. It is found that this is due to bug in compiler. > There is a workaround for this. If only types of deletion_callback and > insertion_callback is replaced with function pointer, code compiles fine. > Is it OK to upload patch that will fix this issue? Interestingly, simple function pointers turned out to be slower. We can try your patch and see what it is doing to performance on the perf bots. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
