Skip to content

Commit

Permalink
Improve NativeFrame memory footprint by storing data in a flat slice …
Browse files Browse the repository at this point in the history
…and supporting generic data slices. (#315)

This change refactors the Go NativeFrame representation to vastly reduce the memory footprint (by __90%__ in some cases) and CPU usage (~30% in some cases). It does this by:

- Storing data in a flat slice, instead of a nested one (e.g. `[][]uint16`). This helps because the 24 byte slice header for Go slices is very significant here for the inner small slices (e.g. when storing 1-3 uint16s, which is often the case). More details in my comment here: #161 (comment)
- Using generics to allow the Go NativeFrame Data slice use an appropriately sized Go integer based on the bitsAllocated (e.g. have the underlying slice be `uint8`, `uint16`, etc instead of always a full-size `int`).

__This is an API breaking change.__ I'm not sure I love the INativeFrame API and naming, but introducing an interface likely makes it easier to interact with the NativeFrame, without forcing propagation of generic signatures everywhere. It also still preserves access to the underlying concrete integer slice if needed. Having a more efficient default NativeFrame representation is important, so lets get this in and iterate if needed before the next release.

This fixes #291 and addresses a big part of #161.

## Benchmarks

See the PR GitHub Action [here](https://github.com/suyashkumar/dicom/actions/runs/10294852102/job/28493571486?pr=315#step:5:69), with an example run from one of the runs screenshotted below:

Memory reduction:
<img width="1149" alt="Screenshot 2024-08-07 at 10 24 56 PM" src="https://github.com/user-attachments/assets/625ac3fd-e947-400f-9969-9a42ff1539b3">

CPU reduction:
<img width="1185" alt="Screenshot 2024-08-07 at 10 29 16 PM" src="https://github.com/user-attachments/assets/415f4b55-e069-402b-91d7-200bee0babb9">
  • Loading branch information
suyashkumar authored Aug 9, 2024
1 parent b00166e commit cb3864e
Show file tree
Hide file tree
Showing 12 changed files with 697 additions and 246 deletions.
8 changes: 5 additions & 3 deletions element.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,8 +426,10 @@ type PixelDataInfo struct {

// IntentionallyUnprocessed indicates that the PixelData Value was actually
// read (as opposed to skipped over, as in IntentionallySkipped above) and
// blindly placed into RawData (if possible). Writing this element back out
// should work. This will be true if the
// blindly placed into UnprocessedValueData (if possible). Writing this
// element back out using the dicom.Writer API should work.
//
// IntentionallyUnprocessed will be true if the
// dicom.SkipProcessingPixelDataValue flag is set with a PixelData tag.
IntentionallyUnprocessed bool `json:"intentionallyUnprocessed"`
// UnprocessedValueData holds the unprocessed Element value data if
Expand All @@ -453,7 +455,7 @@ func (p *pixelDataValue) String() string {
if p.ParseErr != nil {
return fmt.Sprintf("parseErr err=%s FramesLength=%d Frame[0] size=%d", p.ParseErr.Error(), len(p.Frames), len(p.Frames[0].EncapsulatedData.Data))
}
return fmt.Sprintf("FramesLength=%d FrameSize rows=%d cols=%d", len(p.Frames), p.Frames[0].NativeData.Rows, p.Frames[0].NativeData.Cols)
return fmt.Sprintf("FramesLength=%d FrameSize rows=%d cols=%d", len(p.Frames), p.Frames[0].NativeData.Rows(), p.Frames[0].NativeData.Cols())
}

func (p *pixelDataValue) MarshalJSON() ([]byte, error) {
Expand Down
44 changes: 24 additions & 20 deletions element_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,11 +246,12 @@ func TestElement_Equals(t *testing.T) {
Frames: []*frame.Frame{
{
Encapsulated: false,
NativeData: frame.NativeFrame{
BitsPerSample: 8,
Rows: 2,
Cols: 2,
Data: [][]int{{1}, {2}, {3}, {4}},
NativeData: &frame.NativeFrame[int]{
InternalBitsPerSample: 8,
InternalRows: 2,
InternalCols: 2,
InternalSamplesPerPixel: 1,
RawData: []int{1, 2, 3, 4},
},
},
},
Expand All @@ -260,11 +261,12 @@ func TestElement_Equals(t *testing.T) {
Frames: []*frame.Frame{
{
Encapsulated: false,
NativeData: frame.NativeFrame{
BitsPerSample: 8,
Rows: 2,
Cols: 2,
Data: [][]int{{1}, {2}, {3}, {4}},
NativeData: &frame.NativeFrame[int]{
InternalBitsPerSample: 8,
InternalRows: 2,
InternalCols: 2,
InternalSamplesPerPixel: 1,
RawData: []int{1, 2, 3, 4},
},
},
},
Expand All @@ -278,11 +280,12 @@ func TestElement_Equals(t *testing.T) {
Frames: []*frame.Frame{
{
Encapsulated: false,
NativeData: frame.NativeFrame{
BitsPerSample: 8,
Rows: 2,
Cols: 2,
Data: [][]int{{1}, {2}, {3}, {6}},
NativeData: &frame.NativeFrame[int]{
InternalBitsPerSample: 8,
InternalRows: 2,
InternalCols: 2,
InternalSamplesPerPixel: 1,
RawData: []int{1, 2, 3, 6},
},
},
},
Expand All @@ -292,11 +295,12 @@ func TestElement_Equals(t *testing.T) {
Frames: []*frame.Frame{
{
Encapsulated: false,
NativeData: frame.NativeFrame{
BitsPerSample: 8,
Rows: 2,
Cols: 2,
Data: [][]int{{1}, {2}, {3}, {4}},
NativeData: &frame.NativeFrame[int]{
InternalBitsPerSample: 8,
InternalRows: 2,
InternalCols: 2,
InternalSamplesPerPixel: 1,
RawData: []int{1, 2, 3, 4},
},
},
},
Expand Down
5 changes: 2 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ module github.com/suyashkumar/dicom
go 1.22

require (
github.com/google/go-cmp v0.5.2
github.com/google/go-cmp v0.6.0
golang.org/x/exp v0.0.0-20240525044651-4c93da0ed11d
golang.org/x/text v0.3.8
)

require golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 // indirect
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
github.com/google/go-cmp v0.5.2 h1:X2ev0eStA3AbceY54o37/0PQ/UWqKEiiO2dKL5OPaFM=
github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
golang.org/x/exp v0.0.0-20240525044651-4c93da0ed11d h1:N0hmiNbwsSNwHBAvR3QB5w25pUwH4tK0Y/RltD1j1h4=
golang.org/x/exp v0.0.0-20240525044651-4c93da0ed11d/go.mod h1:XtvwrStGgqGPLc4cjQfWqZHG1YFdYs6swckp8vpsjnc=
golang.org/x/text v0.3.8 h1:nAL+RVCQ9uMn3vJZbV+MRnydTJFPf8qqY42YiA6MrqY=
golang.org/x/text v0.3.8/go.mod h1:E6s5w1FMmriuDzIBO73fBruAKo1PCIq6d2Q6DHfQ8WQ=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
4 changes: 2 additions & 2 deletions pkg/frame/encapsulated.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ func (e *EncapsulatedFrame) GetEncapsulatedFrame() (*EncapsulatedFrame, error) {

// GetNativeFrame returns ErrorFrameTypeNotPresent, because this struct does
// not hold a NativeFrame.
func (e *EncapsulatedFrame) GetNativeFrame() (*NativeFrame, error) {
func (e *EncapsulatedFrame) GetNativeFrame() (INativeFrame, error) {
return nil, ErrorFrameTypeNotPresent
}

// GetImage returns a Go image.Image from the underlying frame.
func (e *EncapsulatedFrame) GetImage() (image.Image, error) {
// Decoding the data to only re-encode it as a JPEG *without* modifications
// Decoding the Data to only re-encode it as a JPEG *without* modifications
// is very inefficient. If all you want to do is write the JPEG to disk,
// you should fetch the EncapsulatedFrame and grab the []byte Data from
// there.
Expand Down
14 changes: 7 additions & 7 deletions pkg/frame/frame.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ var ErrorFrameTypeNotPresent = errors.New("the frame type you requested is not p
type CommonFrame interface {
// GetImage gets this frame as an image.Image. Beware that the underlying frame may perform
// some default rendering and conversions. Operate on the raw NativeFrame or EncapsulatedFrame
// if you need to do some custom rendering work or want the data from the dicom.
// if you need to do some custom rendering work or want the Data from the dicom.
GetImage() (image.Image, error)
// IsEncapsulated indicates if the underlying Frame is an EncapsulatedFrame.
IsEncapsulated() bool
// GetNativeFrame attempts to get the underlying NativeFrame (or returns an error)
GetNativeFrame() (*NativeFrame, error)
GetNativeFrame() (INativeFrame, error)
// GetEncapsulatedFrame attempts to get the underlying EncapsulatedFrame (or returns an error)
GetEncapsulatedFrame() (*EncapsulatedFrame, error)
}
Expand All @@ -33,20 +33,20 @@ type Frame struct {
// Encapsulated indicates whether the underlying frame is encapsulated or
// not.
Encapsulated bool
// EncapsulatedData holds the encapsulated data for this frame if
// EncapsulatedData holds the encapsulated Data for this frame if
// Encapsulated is set to true.
EncapsulatedData EncapsulatedFrame
// NativeData holds the native data for this frame if Encapsulated is set
// NativeData holds the native Data for this frame if Encapsulated is set
// to false.
NativeData NativeFrame
NativeData INativeFrame
}

// IsEncapsulated indicates if the frame is encapsulated or not.
func (f *Frame) IsEncapsulated() bool { return f.Encapsulated }

// GetNativeFrame returns a NativeFrame from this frame. If the underlying frame
// is not a NativeFrame, ErrorFrameTypeNotPresent will be returned.
func (f *Frame) GetNativeFrame() (*NativeFrame, error) {
func (f *Frame) GetNativeFrame() (INativeFrame, error) {
if f.Encapsulated {
return f.EncapsulatedData.GetNativeFrame()
}
Expand Down Expand Up @@ -84,7 +84,7 @@ func (f *Frame) Equals(target *Frame) bool {
if f.Encapsulated && !f.EncapsulatedData.Equals(&target.EncapsulatedData) {
return false
}
if !f.Encapsulated && !f.NativeData.Equals(&target.NativeData) {
if !f.Encapsulated && !f.NativeData.Equals(target.NativeData) {
return false
}
return true
Expand Down
Loading

0 comments on commit cb3864e

Please sign in to comment.