Skip to content

Commit

Permalink
Reduce allocations during readNativeFrames (#157)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
suyashkumar authored Dec 19, 2020
1 parent 2745470 commit 33d864f
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 6 deletions.
7 changes: 7 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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 -
10 changes: 5 additions & 5 deletions read.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion read_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
Expand Down

0 comments on commit 33d864f

Please sign in to comment.