Add C# language

[jfheins]

Hi,
I thought you might like to add C# to the mix. I used .net 6 so it should be cross platform and run under linux, mac and windows.

If you think C# is a valuable addition, I will continue to port the java benchmarks over. Maybe 2 questions on the code I already wrote:

• Does "System.Numerics.Vector" count as a undesired data structure? (It does give a 20% speedup)
• I refactored the momentum calculation to bodies.Aggregate(new Vector<double>(), (m, b) => m + b.Velocity * b.Mass); which uses a lambda but is less identical to java. ok or revert?

Just trying to get a feeling for your goals before I tacke the other benchmarks.

[smarr]

Hi @jfheins, yes, the C# benchmarks would be very valuable.

I do have some initial versions of the following already:

• Bounce
• List
• Permute
• Queens
• Sieve
• Storage
• Towers

Does "System.Numerics.Vector" count as a undesired data structure? (It does give a 20% speedup)

Yes, I would avoid it. The idea would be to use data structures that are common to "all" languages. So, we would want to restrict ourselves to plain arrays where possible.

For the NBody benchmark, it is also surprising to me that the compiler can't figure it out.

I refactored the momentum calculation to bodies.Aggregate(new Vector(), (m, b) => m + b.Velocity * b.Mass); which uses a lambda but is less identical to java. ok or revert?

When in doubt about these things, I'd suggest to look at the range of choices we make for other languages. Though, I'd probably want to stick with what Java does. I'd also would want to keep each of the coordinate dimensions as a separate property. Your choice to turn them into vectors prevents use to assess the getter/setter performance, which is one of the main bits measured by the NBody benchmark.

[jfheins]

I ported Richards now as well, I think this could go in if style is fine. I saw the csharp branch, should I try to merge the two or would you rather do it yourself?

[smarr]

A few comments. There are some things that do not seem to match the original implementation and may cause undesired aspects to be included in any comparison.

[smarr]

Please don't change this. It's not a bug.

[jfheins]

will revert

[smarr]

Hm, perhaps the namespace could be AreWeFastYet or so?

[smarr]

Is this file strictly needed?

[jfheins]

It freezes linter settings but ... not stricly needed, no.
Java has a .checkstyle_checks.xml that seems similar ...?

[smarr]

Ah, ok, which linter are you using, and would it be possible to add it to the GitHub Actions setup?
If it's for a linter, having it would be good, yes.
I was just wondering what it was since the file name is .editorconfig instead of check style, or lintrc or similar.

Thanks!

[jfheins]

I was using the Visual Studio 2022 integrated hints, like these:

For better linting (especially reformatting) we are using Resharper at my main job. The cli tool is free of charge and can be integrated into github actions. (and it can load the editorconfig settings)

[smarr]

Thanks, if Resharper is an option, would be great to have similar to CheckStyle, PyLint, RuboCop and the other linter setups :)

[smarr]

Is the I here required by naming convention?
Benchmark shouldn't be an interface but a class.
There's a concrete method in the Benchmark class.

[jfheins]

I is naming convention for an Interface, yes. But I can make Benchmark an abstract class (ideally generic in return type to avoid the object casting) and then it will be Benchmark.

[jfheins]

I think it makes sense to have both the interface and the abstract class. nbody implements the two abstract methods by throwing, this is not ideal. My suggestion would be to have an interface with the inner loop and an abstract class with the loop implemented:

public interface IBenchmark
{
    public bool InnerBenchmarkLoop(int innerIterations);
}

public abstract class Benchmark : IBenchmark
{
    public abstract object Execute();
    public abstract bool VerifyResult(object result);
    public virtual bool InnerBenchmarkLoop(int innerIterations)
    {
        for (int i = 0; i < innerIterations; i++)
        {
            if (!VerifyResult(Execute()))
            {
                return false;
            }
        }
        return true;
    }
}

[smarr]

Hm, I'd be worried about the implications from a compilation perspective.

This seems to be a change from other implementations, and it's not clear to me that the departure is "necessary" in a sense.

AreWeFastYet only "works" because we are trying to be very consistent, and balance idiomatic constraints with comparability/fairness.
So, ideally, we give each language's compiler/runtime exactly the same challenges, minimizing any deviation that's not forced on us by language differences.

[jfheins]

From a compilers perspective, I think this is a virtual call (callvirt IL instruction) one way or the other. But I see that you are favoring consistency stringly and will change this to be a 1:1 match with Java

[smarr]

It's not sufficient to define this as a public field?

[jfheins]

That would be perfectly sufficient. If I remember correctly, Microsoft favors the usage of properties over fields for public members.

Java also used the property pattern i.e.:

  private double x;
  public double getX() { return x; }
  public void setX(final double x) { this.x = x; }

C# property is the equivalent for Java field with get & set methods

[smarr]

I'll happily defer to your or the linter's judgement of what's idiomatic.

My thought was, especially when compared to Java: in C# you don't need to change the client code when switching from public double X to public double X {...}, so, a field might be fine. But indeed, hopefully the compiler will eliminate the difference for trivial ones already anyway. So, using a full property is also a good test to make sure the compiler actually does it.

[jfheins]

Changing a field to a property in C# is not breaking if (and only if) you can re-compile all the consumers. (or it is source-compatible but binary-breaking)
As far as I know, the binary breaking change is the reason MS recommends properties.

[smarr]

Ah, right, very good reason to go with properties then :)

[smarr]

Please don't abbreviate the benchmark name in the folder name.

It's also not clear that you follow the same code style across benchmarks. Wouldn't it be more consistent to either make all classes separate files, or have all benchmarks each in a single file?

[jfheins]

I had that, but my problem was that C# cannot handle a class and a Sub-Namespace with the same name.
I already had a class "Richards" in the namespace Harness.Benchmarks so I renamed the sub namespace. (Note that namespaces need not correlate with folder structure in C#)

As Richards has much more code than nbody, I felt it might need more structure. If you think they should use the same style, which one would you prefer?

[smarr]

Yeah, it's more code, but if we can have a single file per benchmark that would match better what we have for other benchmarks. So, it's a bit of an attempt to get cross-language consistency. Helps with maintaining all the languages... especially when bugs are found, which gets tedious with the increasing number of languages :)

So, avoiding variation is a good thing to keep things going.

[smarr]

Please stick with how Benchmark is implemented in other languages, or in the CSharp branch.

[jfheins]

fixed

[jfheins]

I merged your branch in to extend the test suite - I hope that's ok? (The brnach also contained some changes outside of the CSharp folder)

In the Harness, I added a Dictionary that keeps track of all known Benchmarks, this might be a tad longer but it is also less "magic" than using Reflection so I'd expect more maintainable for you. (Reflection was easy to break with the namespaces)

[smarr]

I did rebase the PR and started setting up ReSharper as well as basic compiling and running on GitHub Actions. May try to look at the current benchmarks, some style issues.

[nikolahua]

@smarr Is this PR eligible to merge?

[smarr]

@smarr Is this PR eligible to merge?

Why do you ask? I had started the last missing benchmark, Havlak, a while ago, but its still not finished. So, the PR is not quite ready yet.

If you would like to contribute, you're very welcome.

If you have a specific use case, it would be great to know, too.

[nikolahua]

@smarr Is this PR eligible to merge?

Why do you ask? I had started the last missing benchmark, Havlak, a while ago, but its still not finished. So, the PR is not quite ready yet.

If you would like to contribute, you're very welcome.

If you have a specific use case, it would be great to know, too.

I have been looking for benchmark suite which is used to measure performance between different versions of .NET runtime for a long time. I've got nothing until I find this answer. However, the results given by this answer seem to be based on the programming language named Oberon. It will become a very valuable benchmark suite for .NET if the C# benchmarks are also added in.

[smarr]

I see. Though, the AreWeFastYet benchmark suite is not perfect for doing comparisons of runtimes of the same language, or rather different versions of the same runtime.

While it can be used for that, it's really just covering a very small part of the runtime system. Thus, one would be better off to have C# benchmarks that are much larger, and test common application behavior.

In the absence of that, finishing up this PR would be a valuable contribution.