My own C# implementation

[hamarb123]

Description

Another prime sieve implementation in C#. The fastest C# base algorithm sieve on my machine.

Contributing requirements

• I read the contribution guidelines in CONTRIBUTING.md.
• I placed my solution in the correct solution folder.
• I added a README.md with the right badge(s).
• I added a Dockerfile that builds and runs my solution.
• I selected drag-race as the target branch.
• All code herein is licensed compatible with BSD-3.

[rbergen]

I'm going to look at this more detail at some point in the next few days, but I can already indicate that we cannot consider this is a faithful C# implementation, and possibly not even a C# implementation at all.

Your solution contains unsafe code and uses hardware-platform specific intrinsics that break C#'s regular platform independence. The .NET runtime technically allows you to do this, but it's similar to having a C program that does a significant part of its processing using inline assembly; that wouldn't be considered a C solution, either. Maybe this solution should indeed be considered a ".NET runtime" implementation, instead of a C# one; the issue with that is that ".NET runtime" is not a language.

As indicated, I will take some more time to make up my mind about this.

[hamarb123]

Hi, I can change the implementations that are vectorized to 'unfaithful' (these should be the only ones that use hardware-platform specific intrinsics) if you want, but I don't see it indicated in the rules that this makes it unfaithful.

[rbergen]

One of the interesting things about a project like this is that it's impossible to predict exactly what creative solutions people come up with. That makes that trying to write rules beforehand that cover every scenario is a pointless exercise. It also means that sometimes we just have to make a decision based on what's in front of us. If we do, we will do our best to capture the core of that decision in a rule that can be generally applied.

You said in #318 that you thought you were working on an interesting implementation. That I do already fully agree with.

[tjol]

Hi, I can change the implementations that are vectorized to 'unfaithful' (these should be the only ones that use hardware-platform specific intrinsics) if you want, but I don't see it indicated in the rules that this makes it unfaithful.

One reason it's unfaithful is that you don't use a class to store the sieve state.

[hamarb123]

One reason it's unfaithful is that you don't use a class to store the sieve state.

It is saved in the method, I don't see the point of allocating a class to put fields in it just for the method. I could make it use a struct without much loss of performance (https://michaelscodingspot.com/avoid-gc-pressure/), but that seems like it wouldn't achieve much to me, when in essence, it is just one function that does the same thing every time. Making it into a struct would allow me to seperate initialising, running, and counting, but that's about all it achieves. Microsoft recommendations say "A static class can be used as a convenient container for sets of methods that just operate on input parameters and do not have to get or set any internal instance fields. For example, in the .NET Class Library, the static System.Math class contains methods that perform mathematical operations, without any requirement to store or retrieve data that is unique to a particular instance of the Math class" for .NET.

[tjol]

I don't see the point of allocating a class

I don't make the rules

[hamarb123]

If I need to I can make it use a struct (which is a commonly used type of a type in C#, closer to a C++ class than a C# class is in terms of allocating and moving in memory), but it would only spread out the code, not do much.
I can't change it today, so feel free take your time to determine if I need to change it or not, but changing it to a struct is not a particular issue.

[rbergen]

If I need to I can make it use a struct

In languages that provide a class construct, class is the one that needs to be used. Similar constructs are acceptable in languages that don't offer classes, but do provide similar approaches (like structs in C).

[hamarb123]

I've made it use a class now and I've marked the vectorized algorithms as unfaithful.
I updated the perf stats with it too, they seem to be relatively w/i the margin of error because of the inlining that is probably done by the JIT, and creating one object per loop is probably not too bad for perf.

[bbartels]

If I need to I can make it use a struct

In languages that provide a class construct, class is the one that needs to be used. Similar constructs are acceptable in languages that don't offer classes, but do provide similar approaches (like structs in C).

From the official rules:

It uses a class to encapsulate the sieve, or an equivalent feature in your language. This class must contain the full state of the sieve. Each iteration should re-create a new instance of this class.

This does not seem to rule out structs, no? Structs are essentially analogous to classes, they just differ in terms of allocation characteristics. So I feel like that would qualify as an 'equivalent feature'.

[rbergen]

I've just opened #398 to clarify this part of the ruleset.

[hamarb123]

Hi, I did make it use a class 4 days ago and marked the vectorized algorithms as unfaithful. Was there anything else I needed to change?

[rbergen]

Nothing I could point at, right now. It's a big PR that aims to add another solution to one of the initial 3 languages for which we already have 3 solutions in place. That means reviewing takes time, and we have to make sure we understand the merits of merging it in its current form. That in itself takes time, too.
If you look at the list of open PRs, this is not the only one by far where this is at play.

[rbergen]

I have a few requests after my review:

• Looking at the code of PrimeSieve.InitializeSieveOptimized(), it seems like the vectorize parameter/field is not actually used for type 1 sieves, meaning methods 2 and 3 are effectively the same (and not vectorized). If my interpretation is correct, please remove the current method 3 from the benchmark run.
• Please shorten the labels for your implementations, and refrain from using commas between them. I would suggest the following translation map, and replacing the commas with underscores (_): standard => std, optimized => opt, vectorized => vct, unmanaged => umgd. You can of course choose an alternative translation if you like, but I think each implementation property should not take more than 3 or 4 characters.
• At least for the faithful implementations, please return the prime candidate array from the RunSieve() method.

[hamarb123]

Hi, I will fix it in a few hours. I'll make the method return whatever internal object is used for storage as an object.
Relating to PrimeSieve.InitializeSieveOptimized(), the vectorisation is used for counting only, but does provide a substantial performance boost (~2x) see perfs. The reason it is this much faster is because it can do 8 at once, and also there is no ifs.
I also think I might seperate each of the 4 types of algorithms into different files, but if I do this, I'll do it in 2 commits, with one purely separating it, so the diff for the other changes is easy to understand.

[hamarb123]

I've separated the algorithms into seperate files, I'm going to implement the rename and update the perf stats in another commit after I re-run the sieve.

[rbergen]

Relating to PrimeSieve.InitializeSieveOptimized(), the vectorisation is used for counting only, but does provide a substantial performance boost (~2x) see perfs.

Please note that it's not necessary to validate the prime count for every iteration in the timed loops; in fact the vast majority of solutions don't. It is good practice to validate the algorithm's outcome once, and in many cases that is done by counting and validating the result of the last iteration, immediate after the timed loop has exited. Of course, in this case that would mean that the sieve variable needs to be declared outside of the loops.

I also think I might seperate each of the 4 types of algorithms into different files

That's appreciated, as I can already confirm that it makes it much easier to review your solution.

[hamarb123]

Hi, so just checking that I could do something like this:

while (condition)
{
	using var sieve = Sieve.Create(method, max);
	sieve.RunSieve();
	passes++;
}
watch.Stop();
{
	using var sieve = Sieve.Create(method, max);
	sieve.RunSieve();
	nint value = sieve.CountPrimes();
	if (value != 78498)
	{
		//error
	}
}

Because that would have a significant improvement on performance (mainly for the currently slower algorithms).
Or if I put it before, I could get rid of the pre-jitting code.

[rbergen]

Hi, so just checking that I could do something like this: [...]
Or if I put it before, I could get rid of the pre-jitting code.

Either is fine, and I've seen both been done before.

[rbergen]

@hamarb123 Do you still want to move forward towards merging this PR?

[hamarb123]

@hamarb123 Do you still want to move forward towards merging this PR?

Sorry, I've been busy lately and my laptop has been running low on storage (causing issues for docker). I'll get it done at some point, I'll convert it into a draft pr for now and ping you when I've finished it.

[marghidanu]

Please tick the checkboxes correctly, use a x instead of v.

[rbergen]

@hamarb123 I understand. That's not a problem, but I will then close this PR for now. You can simply reopen it when you've brought it to a point that it's ready for review.