Change ByteBufferEnumerator<T>.Create(...).ToArray() to IO.ByteConver…

[IanYates]

…ter.ToArray(...) throughout the codebase

I think we could still improve on the code in DicomElement.cs since, in several of the cases, .ToArray() is being called to then just throw away the array to fetch a single item. This change is certainly no worse than the old code (and is indeed better perf-wise) but I suspect we could sometimes keep the ByteBufferEnumerator and use Linq to apply .Skip(item-1).First() on the IEnumerable.

I figured I'd jump in and make this change after seeing a bit of a discussion in gitter.

[IanYates]

@anders9ustafsson - no problem. That'll be a few days away unfortunately.

I'll take the approach of verifying that various corner cases of ByteBufferEnumerator<T>.Create(Buffer).ToArray() produce identical output to IO.ByteConverter.ToArray<T>(Buffer).

This will be easier than trying to create unit tests that exercise the many paths through the changed lines of code and it's closer to what this PR is about anyway (replacing one way of converting bytes to a typed array with another).

[hdesouky]

I have one thing to add here, using MemoryMappedFile instead of heap memory will enable fo-dicom with large studies (tri-phasic and angiography) without requiring high pressure on memory.

I created a special implementation of IByteBuffer using memory mapped files and it is working great in production.

My concern is I don't think this will not work with portable version of the library (using portable libraries)

What do you think? Do you like to incorporate this into consideration?

[anders9ustafsson]

@hdesouky Many thanks for taking portability into account when proposing new solutions, you know I really appreciate that :-)

I don't think portability should stand in the way for good solutions for specific platforms, though, there are always workarounds (better or worse) when porting to other platforms. If your solution improves .NET performance and you are able to share it in this repo, I think that is great!

[IanYates]

Sounds quite clever and should help the GC when there are big files being worked on.

Could we factor this out into a simple replaceable strategy/implementation? The library has the portable default built in but it can be substituted for the memory-mapped version with the addition of some extra code in your project or by including an additional assembly containing that code (similar to what we're proposing for logging).

@hdesouky - does this sound like it might work with your memory-mapped file implementation or did it require some deeper changes to work?

[IanYates]

Closing for new branching model.