Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(766)

Issue 1315173007: Deactivate Parser Bookmarks (Closed)

Created:
5 years, 3 months ago by Michael Hablich
Modified:
5 years, 3 months ago
CC:
v8-dev
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Deactivate Parser Bookmarks. Bookmarks may create a race condition which results in syntax errors. The more files are parsed in parallel the higher the probability that the error occurs. Unfortunately it is not possible to simply revert the CLs related to Bookmarks. BUG=chromium:527930, chromium:510825 LOG=Y Committed: https://crrev.com/129593b40eb69d93ba626601bfda046a95cda0e7 Cr-Commit-Position: refs/heads/master@{#30594}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -2 lines) Patch
M src/parser.cc View 1 chunk +3 lines, -2 lines 0 comments Download

Messages

Total messages: 17 (6 generated)
Michael Hablich
PTAL
5 years, 3 months ago (2015-09-04 11:00:45 UTC) #2
Jakob Kummerow
LGTM, but I'd prefer if vogelheim@ could take a look too.
5 years, 3 months ago (2015-09-04 11:21:05 UTC) #3
Michael Hablich
On 2015/09/04 11:21:05, Jakob wrote: > LGTM, but I'd prefer if vogelheim@ could take a ...
5 years, 3 months ago (2015-09-04 11:58:52 UTC) #4
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1315173007/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1315173007/1
5 years, 3 months ago (2015-09-04 11:59:18 UTC) #6
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: v8_presubmit on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/5553)
5 years, 3 months ago (2015-09-04 12:01:33 UTC) #8
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1315173007/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1315173007/1
5 years, 3 months ago (2015-09-04 13:12:27 UTC) #10
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 3 months ago (2015-09-04 13:53:24 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1315173007/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1315173007/1
5 years, 3 months ago (2015-09-04 16:08:03 UTC) #14
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 3 months ago (2015-09-04 16:15:41 UTC) #15
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/129593b40eb69d93ba626601bfda046a95cda0e7 Cr-Commit-Position: refs/heads/master@{#30594}
5 years, 3 months ago (2015-09-04 16:16:00 UTC) #16
Michael Hablich
5 years, 3 months ago (2015-09-07 07:49:39 UTC) #17
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/1304413007/ by hablich@chromium.org.

The reason for reverting is: Tanks performance (Mandreel latency). A simple
deactivation will not work..

Powered by Google App Engine
This is Rietveld 408576698