Skip to content

Implement CPU benchmark project - #237

Merged
kodiakhq[bot] merged 1 commit into
petabridge:devfrom
Arkatufus:Add_CPU_benchmark
Jan 21, 2022
Merged

kodiakhq[bot] merged 1 commit into
petabridge:devfrom
Arkatufus:Add_CPU_benchmark

Conversation

@Arkatufus

Copy link
Copy Markdown
Contributor

No description provided.

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Left some comments - could you also attach some sample output to this PR?


<ItemGroup>
<PackageReference Include="Tmds.ExecFunction" Version="0.5.0" />
<PackageReference Include="Universe.CpuUsage" Version="2.2.497.598" />

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice, this was the package we mentioned in akkadotnet/akka.net#5400 (comment)

return ExecFunction.Program.Main(args);

// Start lighthouse
var service = new LighthouseService(LighthouseAddress, LighthousePort, ActorSystemName);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So we start 1 Lighthouse node in-proc and another remotely via ExecFunction?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Right now, its one lighthouse in the main benchmark program, and then it will spawn a cluster that uses said lighthose service. Right now it is spawning a cluster with 10 nodes, all using the lighthouse as the seed node.

// Warm up
foreach (var _ in Enumerable.Range(0, 5))
{
CpuUsage.GetByProcess();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Out of curiosity - how does this know which processes to profile? Does it just automatically profile child processes?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Internally, CpuUsage uses an extern call to this method in kernel32.dll:
https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-getprocesstimes

It only handle a single process, most probably not accounting the children processes, and CpuUsage automatically inserts the current process handler when calling the method

}

// Wait until things settles down
await Task.Delay(TimeSpan.FromSeconds(TestDelay));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Makes sense - wait until the cluster is formed.

Log2(N) + 1 + [some fudge factor] is how many seconds it should take a cluster to form, where N is your target cluster size, FYI.

//if (!string.IsNullOrEmpty(ipAddress)) Environment.SetEnvironmentVariable("CLUSTER_IP", ipAddress);

//if (specifiedPort != null)
// Environment.SetEnvironmentVariable("CLUSTER_PORT", specifiedPort.Value.ToString());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We probably need to add this back?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I already added it back, but instead of injecting it into the environment variable, I did a config fallback instead

@kodiakhq
kodiakhq Bot merged commit 9a9b937 into petabridge:dev Jan 21, 2022
@Aaronontheweb

Copy link
Copy Markdown
Member

Uhhhh.... I guess our merge bot merges stuff even though I only commented, not approved.... Good to know I guess.

Arkatufus added a commit that referenced this pull request Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants