SciSharp / NumSharp

High Performance Computation for N-D Tensors in .NET, similar API to NumPy.
https://github.com/SciSharp
Apache License 2.0
1.37k stars 192 forks source link

Dropping implementation of ArrayStorage #294

Closed Nucs closed 5 years ago

Nucs commented 5 years ago

After a deep look at the backend architecture and doing some experiments, I would like you to consider the following benchmark:

image

public double[] genericArray;
public Array nonGenericArray;

[GlobalSetup]
public void Setup()
{
    var rnd = new Random(42);
    // first array
    nonGenericArray = genericArray = new double[10_000_000];

    for (int i = 0; i < genericArray.Length; i++)
    {
        genericArray[i] = rnd.NextDouble();
    }
}

[Benchmark(Baseline = true)]
public void GenericAccess()
{
    var length = genericArray.Length;
    for (int i = 0; i < length; i++)
    {
        var a = genericArray[i];
    }
}

[Benchmark]
public void NonGenericAccess()
{
    double sum = 0;
    var length = nonGenericArray.Length;
    for (int i = 0; i < length; i++)
    {
        var a = nonGenericArray.GetValue(i);
    }
}

Proposition

I propose to drop-off non-generic ArrayStorage and only use TypedGenericStorage.

I have already done it in my local but I can't remove base classes without other architects's review.

Oceania2018 commented 5 years ago

@Nucs Could you push GenericAccess into repo ? I can't find it.

image

henon commented 5 years ago

My 2 cents:

Why the hell not ;)

This is a huge change though. Can you pull it off?

Nucs commented 5 years ago

@henon, Already did (thanks to regen), looking for approval. I can't PR yet but I can't continue without the approval either. And yes, existing tests already pass. @Oceania2018 Added.

Oceania2018 commented 5 years ago

Just push to your repo. Github will put all commits in one PR automatically.

henon commented 5 years ago

Awesome @Nucs, I am looking forward to the PR.

Oceania2018 commented 5 years ago

@Nucs Thank you for your nice comparison, actually, We're using the TypedArrayStorage not ArrayStorage, I've already found the big performance difference before.

I refactored it in TypedArrayStorage.

image

Our long term goal is implement another Storage -> ByteStorage to implement the way of zero-copy, and share data with other libraries like ArrayFire, Apache Arrow.

Nucs commented 5 years ago

Was dropped and successfully replaced with TypedArrayStorage.