Commit 72c8f3fb authored by Mike Stanton's avatar Mike Stanton Committed by V8 LUCI CQ

[compiler] Address two concurrency TODOs

JSHeapBroker::ReadFeedbackForCall() - it may be that the JSFunction
we read in the feedback vector hasn't been store-ordered and is
therefore unsafe to read. Therefore, we need to call the gc
predicate to ensure safety.

JSFunctionRef::feedback_vector() & raw_feedback_cell() - I was able
to remove the TODO warning about uninitialized data visible from
a direct read of these fields from the background. This is because
we either store-order into those fields, or rely on a prior
store-ordering. Additionally, FeedbackVectorRef and FeedbackCellRef
are never-serialized objects, so their first encounter on the
background thread is fine (we don't need to have seen and
serialized them on the main thread first).

Bug: v8:7790
Change-Id: I9cd19999e70fadcf62778dac2b0f679966a4a53f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3026708Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Commit-Queue: Michael Stanton <mvstanton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75720}
parent d5b27bd2
......@@ -423,11 +423,6 @@ class V8_EXPORT_PRIVATE JSFunctionRef : public JSObjectRef {
void SerializeCodeAndFeedback();
bool serialized_code_and_feedback() const;
// The following are available only after calling SerializeCodeAndFeedback().
// TODO(mvstanton): Once we allow inlining of functions we didn't see
// during serialization, we do need to ensure that any feedback vector
// we read here has been fully initialized (ie, store-ordered into the
// cell).
FeedbackVectorRef feedback_vector() const;
FeedbackCellRef raw_feedback_cell() const;
CodeRef code() const;
......
......@@ -804,16 +804,12 @@ ProcessedFeedback const& JSHeapBroker::ReadFeedbackForCall(
base::Optional<HeapObjectRef> target_ref;
{
// TODO(mvstanton): this read has a special danger when done on the
// background thread, because the CallIC has a site in generated code
// where a JSFunction is installed in this slot without store ordering.
// Therefore, we will need to check {maybe_target} to ensure that it
// has been store ordered by the heap's mechanism for store-ordering
// batches of new objects.
MaybeObject maybe_target = nexus.GetFeedback();
HeapObject target_object;
if (maybe_target->GetHeapObject(&target_object)) {
target_ref = MakeRef(this, handle(target_object, isolate()));
// TryMakeRef is used because the GC predicate may fail if the
// JSFunction was allocated too recently to be store-ordered.
target_ref = TryMakeRef(this, handle(target_object, isolate()));
}
}
float frequency = nexus.ComputeCallFrequency();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment