Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace all buffer allocations with pools, Span<T> over R's memory #4

Open
buybackoff opened this issue Feb 22, 2017 · 0 comments
Open

Comments

@buybackoff
Copy link
Member

buybackoff commented Feb 22, 2017

There are several places with new byte[] and new double[] that need pooling.

Also, the code like below needs modern-era rewrite with Unsafe.Read, Unsafe.CopyBlk or even SIMD (see corefxlab/Kestrel) if the IL opcode doesn't vectorize internally.

var data = new byte[sizeof(double)];
            for (int byteIndex = 0; byteIndex < data.Length; byteIndex++)
            {
                data[byteIndex] = Marshal.ReadByte(pointer, offset + byteIndex);
            }
            return BitConverter.ToDouble(data, 0);

There are probably other low-hanging fruits such as mentioned above.

Consider using Span and/or OwnedMemory backed by R's unmanaged memory.

Try to avoid changing Marshal methods to Unsafe just for the sake of it. Only obvious cases such as reading a single double via a loop over its bytes. Later could review all Marshal usages.

Review ArrayConvertAllTwoDim method in ArrayConverter. I believe there is a simpler way to convert 1D <-> 2D array using System.Array.

P.S. For Spreads, any R usage is considered slow, i.e. not an online algo, but batch processing or complex math. So better be safe and do not break things, rather profile remaining non-obvious cases and fix them ad-hoc.

@buybackoff buybackoff changed the title Replace all buffer allocations with pools Replace all buffer allocations with pools, Span<T> over R's memory Feb 22, 2017
buybackoff added a commit that referenced this issue Feb 22, 2017
buybackoff added a commit that referenced this issue Feb 22, 2017
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

No branches or pull requests

1 participant