Skip to content

Handle RequestCores returning 0 in TaskHostFactory tasks #125276

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
lewing wants to merge 1 commit into
base: main
Choose a base branch
from
+60 −36
Draft

Handle RequestCores returning 0 in TaskHostFactory tasks #125276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 15 additions & 9 deletions src/tasks/AotCompilerTask/MonoAOTCompiler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,16 +541,21 @@ private bool ExecuteInternal()
else
{
int allowedParallelism = DisableParallelAot ? 1 : Math.Min(_assembliesToCompile.Count, Environment.ProcessorCount);
int coresAcquired = 0;
IBuildEngine9? be9 = BuildEngine as IBuildEngine9;
try
{
if (be9 is not null)
allowedParallelism = be9.RequestCores(allowedParallelism);
}
catch (NotImplementedException)
if (be9 is not null)
{
Comment on lines 543 to 547
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This RequestCores/ReleaseCores + fallback pattern is now duplicated across several tasks (EmccCompile, ILStrip, EmitBundleBase, MonoAOTCompiler). Since these task projects already compile in src/tasks/Common/Utils.cs, consider extracting a small shared helper (e.g., a method that returns (allowedParallelism, coresAcquired) and handles the fallback) to reduce future drift and make it easier to adjust behavior consistently if MSBuild changes again.

Copilot uses AI. Check for mistakes.
// RequestCores is not implemented in TaskHostFactory
be9 = null;
try
{
coresAcquired = be9.RequestCores(allowedParallelism);
if (coresAcquired > 0)
allowedParallelism = coresAcquired;
}
catch
{
// RequestCores may not be supported in all host environments
Comment on lines +554 to +556
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid a bare catch here. Swallowing all exceptions from RequestCores without logging can hide real issues and make failures harder to debug. Prefer catch (Exception ex) (or known expected exceptions) and log at Low importance before falling back to the original parallelism.

Suggested change
catch
{
// RequestCores may not be supported in all host environments
catch (Exception ex)
{
// RequestCores may not be supported in all host environments
Log?.LogMessage(MessageImportance.Low, $"RequestCores failed; falling back to default parallelism. Exception: {ex}");

Copilot uses AI. Check for mistakes.
coresAcquired = 0;
}
}

/*
Expand Down Expand Up @@ -598,7 +603,8 @@ all assigned to that one partition.
}
finally
{
be9?.ReleaseCores(allowedParallelism);
if (coresAcquired > 0)
be9?.ReleaseCores(coresAcquired);
}
}

Expand Down
24 changes: 15 additions & 9 deletions src/tasks/MonoTargetsTasks/EmitBundleTask/EmitBundleBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,16 +154,21 @@ public override bool Execute()

// Generate source file(s) containing each resource's byte data and size
int allowedParallelism = Math.Max(Math.Min(bundledResources.Count, Environment.ProcessorCount), 1);
int coresAcquired = 0;
IBuildEngine9? be9 = BuildEngine as IBuildEngine9;
try
{
if (be9 is not null)
allowedParallelism = be9.RequestCores(allowedParallelism);
}
catch (NotImplementedException)
if (be9 is not null)
{
// RequestCores is not implemented in TaskHostFactory
be9 = null;
try
{
coresAcquired = be9.RequestCores(allowedParallelism);
if (coresAcquired > 0)
allowedParallelism = coresAcquired;
}
catch
{
// RequestCores may not be supported in all host environments
Comment on lines +167 to +169
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid a bare catch around RequestCores. Catching everything without logging can mask unexpected failures and complicate diagnosing build issues. Prefer catching Exception ex (or known exceptions) and log at Low importance before proceeding with the fallback parallelism.

Suggested change
catch
{
// RequestCores may not be supported in all host environments
catch (Exception ex)
{
// RequestCores may not be supported in all host environments
Log.LogMessage(
MessageImportance.Low,
"RequestCores({0}) failed: {1}",
allowedParallelism,
ex.Message);

Copilot uses AI. Check for mistakes.
coresAcquired = 0;
}
}

try
Expand Down Expand Up @@ -196,7 +201,8 @@ public override bool Execute()
}
finally
{
be9?.ReleaseCores(allowedParallelism);
if (coresAcquired > 0)
be9?.ReleaseCores(coresAcquired);
}

foreach (ITaskItem bundledResource in bundledResources)
Expand Down
24 changes: 15 additions & 9 deletions src/tasks/MonoTargetsTasks/ILStrip/ILStrip.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,16 +74,21 @@ public override bool Execute()
Log.LogMessage(MessageImportance.High, "IL stripping assemblies");

int allowedParallelism = DisableParallelStripping ? 1 : Math.Min(Assemblies.Length, Environment.ProcessorCount);
int coresAcquired = 0;
IBuildEngine9? be9 = BuildEngine as IBuildEngine9;
try
{
if (be9 is not null)
allowedParallelism = be9.RequestCores(allowedParallelism);
}
catch (NotImplementedException)
if (be9 is not null)
{
// RequestCores is not implemented in TaskHostFactory
be9 = null;
try
{
coresAcquired = be9.RequestCores(allowedParallelism);
if (coresAcquired > 0)
allowedParallelism = coresAcquired;
}
catch
{
// RequestCores may not be supported in all host environments
Comment on lines +87 to +89
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid a bare catch here. It suppresses all exceptions from RequestCores without any trace, which can hide real problems. Prefer catch (Exception ex) (or a small set of expected exceptions) and log at Low importance before falling back.

Suggested change
catch
{
// RequestCores may not be supported in all host environments
catch (Exception ex)
{
// RequestCores may not be supported in all host environments
Log.LogMessage(
MessageImportance.Low,
"RequestCores failed; falling back to default parallelism. Exception: {0}",
ex);

Copilot uses AI. Check for mistakes.
coresAcquired = 0;
}
}

try
Expand Down Expand Up @@ -113,7 +118,8 @@ public override bool Execute()
}
finally
{
be9?.ReleaseCores(allowedParallelism);
if (coresAcquired > 0)
be9?.ReleaseCores(coresAcquired);
}

return !Log.HasLoggedErrors;
Expand Down
24 changes: 15 additions & 9 deletions src/tasks/WasmAppBuilder/EmccCompile.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,16 +131,21 @@ private bool ExecuteActual()
Directory.CreateDirectory(_tempPath);

int allowedParallelism = DisableParallelCompile ? 1 : Math.Min(SourceFiles.Length, Environment.ProcessorCount);
int coresAcquired = 0;
IBuildEngine9? be9 = BuildEngine as IBuildEngine9;
try
{
if (be9 is not null)
allowedParallelism = be9.RequestCores(allowedParallelism);
}
catch (NotImplementedException)
if (be9 is not null)
{
// RequestCores is not implemented in TaskHostFactory
be9 = null;
try
{
coresAcquired = be9.RequestCores(allowedParallelism);
if (coresAcquired > 0)
allowedParallelism = coresAcquired;
}
catch
{
// RequestCores may not be supported in all host environments
Comment on lines +144 to +146
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid a bare catch here. This will swallow unexpected exceptions from RequestCores (e.g., MSBuild bugs / environment failures) without any logging, which can make build failures very hard to diagnose. Catch Exception ex (or a narrower set of expected exceptions) and consider logging at Low importance before falling back.

Suggested change
catch
{
// RequestCores may not be supported in all host environments
catch (Exception ex)
{
// RequestCores may not be supported in all host environments
Log.LogMessage(MessageImportance.Low, $"RequestCores({allowedParallelism}) failed, falling back to default parallelism. Exception: {ex.Message}");

Copilot uses AI. Check for mistakes.
coresAcquired = 0;
}
}

/*
Expand Down Expand Up @@ -184,7 +189,8 @@ all assigned to that one partition.
}
finally
{
be9?.ReleaseCores(allowedParallelism);
if (coresAcquired > 0)
be9?.ReleaseCores(coresAcquired);
}


Expand Down
Loading