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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1877973002: Invalidate background compilation only if relevant fields were invalidated. (Closed)

Created:
4 years, 8 months ago by srdjan
Modified:
4 years, 8 months ago
Reviewers:
Florian Schneider, siva
CC:
reviews_dartlang.org, vm-dev_dartlang.org
Base URL:
git@github.com:dart-lang/sdk.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Invalidate background compilation only if relevant fields were invalidated. BUG= R=fschneider@google.com Committed: https://github.com/dart-lang/sdk/commit/cb63ed46fedbd95b082fd6b1ac08682d8dd83e35

Patch Set 1 #

Patch Set 2 : fixes #

Patch Set 3 : sync #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+108 lines, -14 lines) Patch
M runtime/vm/compiler.cc View 1 2 3 chunks +34 lines, -4 lines 4 comments Download
M runtime/vm/isolate.h View 1 2 chunks +12 lines, -2 lines 0 comments Download
M runtime/vm/isolate.cc View 5 chunks +58 lines, -7 lines 0 comments Download
M runtime/vm/object.cc View 1 1 chunk +4 lines, -1 line 0 comments Download

Messages

Total messages: 7 (2 generated)
srdjan
4 years, 8 months ago (2016-04-11 21:50:33 UTC) #2
Florian Schneider
lgtm
4 years, 8 months ago (2016-04-11 22:32:13 UTC) #3
srdjan
Committed patchset #3 (id:40001) manually as cb63ed46fedbd95b082fd6b1ac08682d8dd83e35 (presubmit successful).
4 years, 8 months ago (2016-04-11 22:35:06 UTC) #5
siva
some DBC. https://codereview.chromium.org/1877973002/diff/40001/runtime/vm/compiler.cc File runtime/vm/compiler.cc (right): https://codereview.chromium.org/1877973002/diff/40001/runtime/vm/compiler.cc#newcode385 runtime/vm/compiler.cc:385: Isolate::Current()->ClearDisablingFieldList(); maybe use 'isolate()' here to avoid ...
4 years, 8 months ago (2016-04-12 17:46:45 UTC) #6
srdjan
4 years, 8 months ago (2016-04-12 19:47:19 UTC) #7
Message was sent while issue was closed. 
https://codereview.chromium.org/1877973002/diff/40001/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/1877973002/diff/40001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:385: Isolate::Current()->ClearDisablingFieldList();
On 2016/04/12 17:46:45, siva wrote:
> maybe use 'isolate()' here to avoid another Thread::Current()

Done in next CL.

https://codereview.chromium.org/1877973002/diff/40001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:435: }
On 2016/04/12 17:46:45, siva wrote:
> This loop acquires/release lock with every access to the Disabling Field list
> wouldn't it be better to pass the guarded_fields list into a function
> isolate->CheckDisablingFields that acquires the lock once and checks if any
> field in guarded fields list is present in the disabling field list.

Since this occurs only while background compiler is at a safepoint, will remove
the lock in GetDisablingField and assert that we are at a safepoint.

Powered by Google App Engine
This is Rietveld 408576698