dotnet / dotNext

Next generation API for .NET
https://dotnet.github.io/dotNext/
MIT License
1.57k stars 119 forks source link

[API Proposoal]: Additional methods for "DotNext.Span" #193

Closed CodingMadness closed 8 months ago

CodingMadness commented 8 months ago

Heyo,

I have written some "SpanExtension" methods cause its needed to be optimized for rendering logic and I thought maybe they could go inside your DotNext.Span namespace :)

I will post a github gist "Utils" class where i hord those functions needed for my 2D game.

This gist contains 3 core methods:

this gist here contains the "SpanInfo" type building all the nessecary information pre-hand, so its easier to control what is needed and when to cuse them, debloats heavily the Swap() function

I really hope that you have a look upon them and tell me if they are useful and when not, also tell me pls why ^^

All good 2 you and ur family and really really nice job you did with DotNext!

Cheers!

CodingMadness commented 8 months ago

Sakno, I would also suggest you to not check for, if the Range is >= 0, even the Team from .NET doesnt check for that because its senseless to check smth which should not be computed anyway, a Range is always to be used in collection of which you cant be < 0, so you could spare that check, as I have done in my code.

sakno commented 8 months ago

Sakno, I would also suggest you to not check for, if the Range is >= 0, even the Team from .NET doesnt check for that because its senseless to check smth which should not be computed anyway, a Range is always to be used in collection of which you cant be < 0, so you could spare that check, as I have done in my code.

For which method? For Move<T> I'm not checking for < 0. I'm checking for length is not 0, which is different and reasonable.

CodingMadness commented 8 months ago

@sakno you have a complete benchmarkable project, I would be glad that you run it and see the benchmarks which are now indeed quite worse, okay I am using 1MB string/span and x and y are 1KB in size, for now I test:

There other cases (like checking if they are directly close, or just distinct by 2-3 blocks and so on....) but for now these are off the list which i will add next.

But I feel the worsen in Performance has to do that allocating with MemoryOwner<T> (inside SpanQueue) , because he has to do alot when benchmarking, before i only used stackallocwhich u are right ofc cant be the solution due to potential StackoverflowExcp. But I wonder ....

CodingMadness commented 8 months ago

You have an invite to my private-repo btw! its called "Utility-Workbench".

sakno commented 8 months ago

Unfortunately, I can't provide a review of you code. Probably, the types in your project are suitable and convenient for your task, that's fine. I would like to be focused on API proposal and its implementation which is finished. I don't expected great performance from all these three methods, they are all O(n) operations without any possibility to reduce time complexity (and even space complexity). I think that allocation by itself is not bad, especially it situations when the object is not trying to escape Gen0. Allocation is just a bump of heap pointer, which is more cheaper than rearranging ranges within the same span. Remember, premature optimization is a root of all evil.

CodingMadness commented 8 months ago

Unfortunately, I can't provide a review of you code. Probably, the types in your project are suitable and convenient for your task, that's fine. I would like to be focused on API proposal and its implementation which is finished. I don't expected great performance from all these three methods, they are all O(n) operations without any possibility to reduce time complexity (and even space complexity). I think that allocation by itself is not bad, especially it situations when the object is not trying to escape Gen0. Allocation is just a bump of heap pointer, which is more cheaper than rearranging ranges within the same span. Remember, premature optimization is a root of all evil.

Well, as I said, testing with a anyway ridiculous amount of string-length (1MB large string) and 1KB-2KB sized ranges and they reach to like 25-50ms which is fine i think, and its not about allocation per se, its about to have the choice to take a solution if you dont want any heap allocation cause sometimes you cant just do that especially if u need to do stuff only cpl of times, (as a good example is high throughput apps like games or other programs which need to work on lower RAM)

I mean space-complexity is at 0? or how am I supposed to decipher your meaning of reducing space-complexity?

And for strings/spans with like ~1KB max as source-length this is super fast, which I have tested, (250-550 chars) it uses maxium 200ns with 0 allocation.

And this is a function which is generalized, this is nothing special, it works for every blittable types.

I dont even know why you coded the stuff for yourself when you could have atleast review the code and based on the understanding which I tried to explain you here since a week, you could then do adaptions to it and not write from scratch, I thought atleast this is how collabs is supposed to work.

And doing a replace/realloc of everystring in a frame-based system like Game Dev, where at LOWEST your functions get called 60x/sec up to 200x/sec this is super bad to create new strings, and in my version I use memory rented buffer for that purpose.

But you seemed that you did not want to check my commited stuff at all cost which is all in itself a pretty bad basis for discussion, nevermind, I wont suggest here anything more anyway, I dont think this is valuable of my time.

sakno commented 8 months ago

high throughput apps like games or other programs which need to work on lower RAM

Throughput and RAM are orthogonal. RAM is cheaper, so if I want to increase RAM usage to improve throughput, I definitely will do that. In your case, the cost of computation is moved from RAM to CPU which is not good in general.

I dont even know why you coded the stuff for yourself when you could have atleast review the code

I did that and provided feedback above. Starting from incorrect usage of AddressOf. Also I don't see any value of SpanInfo type, I just don't need it. You can just compare the number of lines and the complexity of your vs my version. There are two options for implementing new API:

Incorporating third-party code from different repo is a bit risky because of licensing. Moreover, if I see that the code requires a lot of rework, I prefer to implement it from scratch. Additionally, proposal should be valuable and reusable. I didn't see how these methods perform better than regular heap allocation. Do you have any evidence that Gen0 allocation is really worse than moving ranges within the span?