From 33d864f317adcb467fbe67cceae3052d585ee8b2 Mon Sep 17 00:00:00 2001 From: Suyash Kumar Date: Sat, 19 Dec 2020 17:24:57 -0500 Subject: [PATCH] Reduce allocations during readNativeFrames (#157) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change reduces the number of allocations in `readNativeFrames`. Instead of allocating a slice for each pixel's samples, a single flat buffer slice is allocated upfront of the size `pixelsPerFrame*samplesPerPixel`. Later, ranges in that slice are referred to in the larger 2D slice. This leads to there only being two calls to `make`, leading to significant performance gains. On my machine running `make bench-diff`: ``` name old time/op new time/op delta Parse/1.dcm-8 2.29ms ± 1% 1.95ms ± 3% -15.00% (p=0.008 n=5+5) Parse/2.dcm-8 2.27ms ± 1% 1.94ms ± 0% -14.41% (p=0.008 n=5+5) Parse/3.dcm-8 2.10ms ± 0% 1.81ms ± 2% -13.77% (p=0.008 n=5+5) Parse/4.dcm-8 240ms ± 1% 196ms ± 4% -18.27% (p=0.008 n=5+5) Parse/5.dcm-8 33.6ms ± 1% 27.9ms ± 0% -17.00% (p=0.008 n=5+5) ``` We see similar results in the [GitHub action benchmark](https://github.com/suyashkumar/dicom/pull/157/checks?check_run_id=1574909456#step:5:26). Presumably the percentage performance gains would be even higher for DICOMs with more Native PixelData (e.g. more frames, pixels per frame, samples per pixel, etc). This helps address #161. --- Makefile | 7 +++++++ read.go | 10 +++++----- read_test.go | 2 +- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index ac1c1910..389c91a1 100644 --- a/Makefile +++ b/Makefile @@ -31,3 +31,10 @@ release: tar -zcvf ${BINARY}-linux-amd64.tar.gz ${BINARY}-linux-amd64; \ tar -zcvf ${BINARY}-darwin-amd64.tar.gz ${BINARY}-darwin-amd64; \ zip -r ${BINARY}-windows-amd64.exe.zip ${BINARY}-windows-amd64.exe; + +bench-diff: + go test -bench . -count 5 > bench_current.txt + git checkout main + go test -bench . -count 5 > bench_main.txt + benchstat bench_main.txt bench_current.txt + git checkout - diff --git a/read.go b/read.go index d0e506a0..14e7fbf0 100644 --- a/read.go +++ b/read.go @@ -223,30 +223,30 @@ func readNativeFrames(d dicomio.Reader, parsedData *Dataset, fc chan<- *frame.Fr Data: make([][]int, int(pixelsPerFrame)), }, } + buf := make([]int, int(pixelsPerFrame)*samplesPerPixel) for pixel := 0; pixel < int(pixelsPerFrame); pixel++ { - currentPixel := make([]int, samplesPerPixel) for value := 0; value < samplesPerPixel; value++ { if bitsAllocated == 8 { val, err := d.ReadUInt8() if err != nil { return nil, bytesRead, err } - currentPixel[value] = int(val) + buf[(pixel*samplesPerPixel)+value] = int(val) } else if bitsAllocated == 16 { val, err := d.ReadUInt16() if err != nil { return nil, bytesRead, err } - currentPixel[value] = int(val) + buf[(pixel*samplesPerPixel)+value] = int(val) } else if bitsAllocated == 32 { val, err := d.ReadUInt32() if err != nil { return nil, bytesRead, err } - currentPixel[value] = int(val) + buf[(pixel*samplesPerPixel)+value] = int(val) } } - currentFrame.NativeData.Data[pixel] = currentPixel + currentFrame.NativeData.Data[pixel] = buf[pixel*samplesPerPixel : (pixel+1)*samplesPerPixel] } image.Frames[frameIdx] = currentFrame if fc != nil { diff --git a/read_test.go b/read_test.go index 03001893..8cba5b3d 100644 --- a/read_test.go +++ b/read_test.go @@ -282,7 +282,7 @@ func TestReadNativeFrames(t *testing.T) { t.Errorf("TestReadNativeFrames(%v): did not get expected error. got: %v, want: %v", tc.data, err, tc.expectedError) } - if diff := cmp.Diff(pixelData, tc.expectedPixelData); diff != "" { + if diff := cmp.Diff(tc.expectedPixelData, pixelData); diff != "" { t.Errorf("TestReadNativeFrames(%v): unexpected diff: %v", tc.data, diff) } })