Commit 63f42852 authored by Clemens Backes's avatar Clemens Backes Committed by V8 LUCI CQ

[wasm] Minor fixes to priority compilation units

This fixes a few minor issues in the handling of priority top-tier
compilation units, and adds some comment.

1) We document the current design around priority top-tier units.
2) We simplify the code to increase the priority a bit, and make sure to
   avoid integer overflows.
3) We reorder two statements to first increase the
   outstanding_top_tier_functions_ counter before adding a new unit, in
   order to avoid starting to execute a top-tier unit when the counter
   is 0, which would be an inconsistent state.

R=ahaas@chromium.org

Bug: v8:12880
Change-Id: I67bd71135f34b793ea5cf064108668fb72c7e345
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3654097Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80634}
parent 6a977bd1
...@@ -239,8 +239,14 @@ class CompilationUnitQueues { ...@@ -239,8 +239,14 @@ class CompilationUnitQueues {
void AddTopTierPriorityUnit(WasmCompilationUnit unit, size_t priority) { void AddTopTierPriorityUnit(WasmCompilationUnit unit, size_t priority) {
base::SharedMutexGuard<base::kShared> queues_guard(&queues_mutex_); base::SharedMutexGuard<base::kShared> queues_guard(&queues_mutex_);
// Add to the individual queues in a round-robin fashion. No special care is // Add to the individual queues in a round-robin fashion. No special care is
// taken to balance them; they will be balanced by work stealing. We use // taken to balance them; they will be balanced by work stealing.
// the same counter for this reason. // Priorities should only be seen as a hint here; without balancing, we
// might pop a unit with lower priority from one queue while other queues
// still hold higher-priority units.
// Since updating priorities in a std::priority_queue is difficult, we just
// add new units with higher priorities, and use the
// {CompilationUnitQueues::top_tier_compiled_} array to discard units for
// functions which are already being compiled.
int queue_to_add = next_queue_to_add.load(std::memory_order_relaxed); int queue_to_add = next_queue_to_add.load(std::memory_order_relaxed);
while (!next_queue_to_add.compare_exchange_weak( while (!next_queue_to_add.compare_exchange_weak(
queue_to_add, next_task_id(queue_to_add, queues_.size()), queue_to_add, next_task_id(queue_to_add, queues_.size()),
...@@ -1440,23 +1446,21 @@ void TriggerTierUp(Isolate* isolate, NativeModule* native_module, ...@@ -1440,23 +1446,21 @@ void TriggerTierUp(Isolate* isolate, NativeModule* native_module,
kNoDebugging}; kNoDebugging};
const WasmModule* module = native_module->module(); const WasmModule* module = native_module->module();
size_t priority; int priority;
{ {
// TODO(clemensb): Try to avoid the MutexGuard here.
base::MutexGuard mutex_guard(&module->type_feedback.mutex); base::MutexGuard mutex_guard(&module->type_feedback.mutex);
int saved_priority = int& stored_priority =
module->type_feedback.feedback_for_function[func_index].tierup_priority; module->type_feedback.feedback_for_function[func_index].tierup_priority;
saved_priority++; if (stored_priority < kMaxInt) ++stored_priority;
module->type_feedback.feedback_for_function[func_index].tierup_priority = priority = stored_priority;
saved_priority;
// Continue to creating a compilation unit if this is the first time
// we detect this function as hot, and create a new higher-priority unit
// if the number of tierup checks is a power of two (at least 4).
if (saved_priority > 1 &&
(saved_priority < 4 || (saved_priority & (saved_priority - 1)) != 0)) {
return;
}
priority = saved_priority;
} }
// Only create a compilation unit if this is the first time we detect this
// function as hot (priority == 1), or if the priority increased
// significantly. The latter is assumed to be the case if the priority
// increased at least to four, and is a power of two.
if (priority == 2 || !base::bits::IsPowerOfTwo(priority)) return;
if (FLAG_wasm_speculative_inlining) { if (FLAG_wasm_speculative_inlining) {
// TODO(jkummerow): we could have collisions here if different instances // TODO(jkummerow): we could have collisions here if different instances
// of the same module have collected different feedback. If that ever // of the same module have collected different feedback. If that ever
...@@ -3489,11 +3493,11 @@ void CompilationStateImpl::CommitTopTierCompilationUnit( ...@@ -3489,11 +3493,11 @@ void CompilationStateImpl::CommitTopTierCompilationUnit(
void CompilationStateImpl::AddTopTierPriorityCompilationUnit( void CompilationStateImpl::AddTopTierPriorityCompilationUnit(
WasmCompilationUnit unit, size_t priority) { WasmCompilationUnit unit, size_t priority) {
compilation_unit_queues_.AddTopTierPriorityUnit(unit, priority);
{ {
base::MutexGuard guard(&callbacks_mutex_); base::MutexGuard guard(&callbacks_mutex_);
outstanding_top_tier_functions_++; outstanding_top_tier_functions_++;
} }
compilation_unit_queues_.AddTopTierPriorityUnit(unit, priority);
compile_job_->NotifyConcurrencyIncrease(); compile_job_->NotifyConcurrencyIncrease();
} }
......
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