Skip to content

Commit

Permalink
Remove the --experimental_prioritize_local_actions command line option.
Browse files Browse the repository at this point in the history
RELNOTES[INC]: The --experimental_prioritize_local_actions command line option is not available anymore.

PiperOrigin-RevId: 568824089
Change-Id: Ie328ae97dac638585414e1c6bf1e45c3d16a8ed4
  • Loading branch information
lberki authored and Copybara-Service committed Sep 27, 2023
1 parent e1eef7c commit e45b8d4
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 99 deletions.
Expand Up @@ -195,9 +195,6 @@ public double getUsedCPU() {
// Used local test count. Corresponds to the local test count definition in the ResourceSet class.
private int usedLocalTestCount;

/** If set, local-only actions are given priority over dynamically run actions. */
private boolean prioritizeLocalActions;

@VisibleForTesting
public static ResourceManager instanceForTestingOnly() {
return new ResourceManager();
Expand Down Expand Up @@ -243,11 +240,6 @@ public void setWorkerPool(WorkerPool workerPool) {
this.workerPool = workerPool;
}

/** Sets whether to prioritize local-only actions in resource allocation. */
public void setPrioritizeLocalActions(boolean prioritizeLocalActions) {
this.prioritizeLocalActions = prioritizeLocalActions;
}

/**
* Acquires requested resource set. Will block if resource is not available. NB! This method must
* be thread-safe!
Expand Down Expand Up @@ -401,24 +393,20 @@ private synchronized LatchWithWorker acquire(ResourceSet resources, ResourcePrio
}
Pair<ResourceSet, LatchWithWorker> request =
new Pair<>(resources, new LatchWithWorker(new CountDownLatch(1), /* worker= */ null));
if (this.prioritizeLocalActions) {
switch (priority) {
case LOCAL:
localRequests.addLast(request);
break;
case DYNAMIC_WORKER:
// Dynamic requests should be LIFO, because we are more likely to win the race on newer
// actions.
dynamicWorkerRequests.addFirst(request);
break;
case DYNAMIC_STANDALONE:
// Dynamic requests should be LIFO, because we are more likely to win the race on newer
// actions.
dynamicStandaloneRequests.addFirst(request);
break;
}
} else {
localRequests.addLast(request);
switch (priority) {
case LOCAL:
localRequests.addLast(request);
break;
case DYNAMIC_WORKER:
// Dynamic requests should be LIFO, because we are more likely to win the race on newer
// actions.
dynamicWorkerRequests.addFirst(request);
break;
case DYNAMIC_STANDALONE:
// Dynamic requests should be LIFO, because we are more likely to win the race on newer
// actions.
dynamicStandaloneRequests.addFirst(request);
break;
}
return request.second;
}
Expand Down
Expand Up @@ -987,7 +987,6 @@ private Builder createBuilder(
@VisibleForTesting
public static void configureResourceManager(ResourceManager resourceMgr, BuildRequest request) {
ExecutionOptions options = request.getOptions(ExecutionOptions.class);
resourceMgr.setPrioritizeLocalActions(options.prioritizeLocalActions);
ImmutableMap<String, Float> extraResources =
options.localExtraResources.stream()
.collect(
Expand Down
Expand Up @@ -355,17 +355,6 @@ public boolean usingLocalTestJobs() {
return localTestJobs != 0;
}

@Option(
name = "experimental_prioritize_local_actions",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.EXECUTION},
help =
"If set, actions that can only run locally are given first chance at acquiring resources,"
+ " dynamically run workers get second chance, and dynamically-run standalone actions"
+ " come last.")
public boolean prioritizeLocalActions;

@Option(
name = "cache_computed_file_digests",
defaultValue = "50000",
Expand Down
Expand Up @@ -79,7 +79,6 @@ public void configureResourceManager() throws Exception {
sync = new CyclicBarrier(2);
sync2 = new CyclicBarrier(2);
rm.resetResourceUsage();
rm.setPrioritizeLocalActions(true);
worker = mock(Worker.class);
rm.setWorkerPool(createWorkerPool());
}
Expand Down Expand Up @@ -515,66 +514,6 @@ public void testOutOfOrderAllocation() throws Exception {
assertThat(rm.inUse()).isFalse();
}

@Test
@SuppressWarnings("ThreadPriorityCheck")
public void testRelease_noPriority() throws Exception {
rm.setPrioritizeLocalActions(false);
assertThat(rm.inUse()).isFalse();

TestThread thread1 =
new TestThread(
() -> {
acquire(700, 0, 0);
sync.await();
sync2.await();
release(700, 0, 0);
});
thread1.start();
// Wait for thread1 to have acquired its RAM
sync.await(1, TimeUnit.SECONDS);

// Set up threads that compete for resources
CyclicBarrier syncDynamicStandalone =
startAcquireReleaseThread(ResourcePriority.DYNAMIC_STANDALONE);
while (rm.getWaitCount() < 1) {
Thread.yield();
}
CyclicBarrier syncDynamicWorker = startAcquireReleaseThread(ResourcePriority.DYNAMIC_WORKER);
while (rm.getWaitCount() < 2) {
Thread.yield();
}
CyclicBarrier syncLocal = startAcquireReleaseThread(ResourcePriority.LOCAL);
while (rm.getWaitCount() < 3) {
Thread.yield();
}

sync2.await();

while (syncLocal.getNumberWaiting()
+ syncDynamicWorker.getNumberWaiting()
+ syncDynamicStandalone.getNumberWaiting()
== 0) {
Thread.yield();
}
assertThat(rm.getWaitCount()).isEqualTo(2);
assertThat(syncDynamicStandalone.getNumberWaiting()).isEqualTo(1);
syncDynamicStandalone.await(1, TimeUnit.SECONDS);

while (syncDynamicWorker.getNumberWaiting() + syncLocal.getNumberWaiting() == 0) {
Thread.yield();
}
assertThat(syncDynamicWorker.getNumberWaiting()).isEqualTo(1);
assertThat(rm.getWaitCount()).isEqualTo(1);

syncDynamicWorker.await(1, TimeUnit.SECONDS);
while (syncLocal.getNumberWaiting() == 0) {
Thread.yield();
}
assertThat(syncLocal.getNumberWaiting()).isEqualTo(1);
assertThat(rm.getWaitCount()).isEqualTo(0);
syncLocal.await(1, TimeUnit.SECONDS);
}

@Test
@SuppressWarnings("ThreadPriorityCheck")
public void testRelease_highPriorityFirst() throws Exception {
Expand Down

0 comments on commit e45b8d4

Please sign in to comment.