aardappel / treesheets

TreeSheets : Free Form Data Organizer (see strlen.com/treesheets)
zlib License
2.49k stars 184 forks source link

Manual memory management on image data #672

Closed tobiolo closed 2 weeks ago

tobiolo commented 2 weeks ago

Replace vector for image data with raw array This allows faster/constant time access to image data

tobiolo commented 2 weeks ago

Put some large 4K images into a TreeSheets and the difference is observable.

aardappel commented 2 weeks ago

Can you point me to where exactly the speedup happens? Because a vector should be completely equivalent to a raw pointer, unless you're sharing these raw pointers or something. And for most purposes the vector is better/safer.

Like, if vector<uint8_t> ConvertWxImageToBuffer(const wxImage &im, wxBitmapType bmt) was slow because it makes a copy on return (it shouldn't, but who knows), then that can be fixed by making the vector an argument instead of return value, etc.

tobiolo commented 2 weeks ago

The speedup happens when the wxBitmap is constructed from the wxImage. If the wxImage is constructed from an array, it seems to be faster. image

tobiolo commented 2 weeks ago

Also tested your proposal, but I did not see any change in the constructor stats, so does not seem to be copied?

aardappel commented 2 weeks ago

Where is InitFromImage called from? I don't see it in the code.

It seems like that call takes a wxImage which has already been constructed at that point, so I don't see how that function can vary..

There is no way that calling .data() and .size() on a vector is any slower than storing these values raw. A vector is just a convenient wrapper around the buffer you created by hand. The only way a vector can be slower is if the code is written to cause it to copy, in which case only that copy needs fixing.

I don't mind using a raw pointer if it is absolutely necessary, but I think only if we know for sure what causes the slowdown.

Worth running on windows with https://github.com/VerySleepy/verysleepy, which may give you better output than gprof

tobiolo commented 2 weeks ago

Well, this seems to be platform specific. image

image

tobiolo commented 2 weeks ago

It seems that you always need to go via wxImage and this something to live with :-). So I am closing this PR for now. The code we are using is the same wxWidgets is using e.g. internally in https://docs.wxwidgets.org/latest/classwx_bitmap.html#a2048897970d5e845040fcfc0259e1be1