xiphonics/picoTracker

Reduce ram usage of Variable objects

Closed this issue · 5 comments

maks commented

Coming out of research done in #17 (comment) the ram usage of Variable objects can be significantly reduced.

The first identified issue per above:

SampleInstrument constructor new's 18 Variable or WatchedVariable member objects each of which has a char string_[40], but as far as I can see that's only used when the Variable is of type STRING which none of those are.

Given 18 variables per sample instrument thats: 16 * 18 * 40 = 11520 bytes potential saving.
Given 5 variable per midi instrument thats: 16 * 5 * 40 = 3200 bytes potential saving.

Next, every Variable object has a name_ member which is a std::string which is assigned in the contructor:

Variable::Variable(const char *name, FourCC id, const char *value) {
  name_ = name;

but the above means the std::string does a copy of the const char into its internal buffer! This means that every Variable object gets a copy of the name string instead of all just pointing to string literal in flash, because Variable object are created all through the codebase like so:

filterMode_ = new Variable("filter mode", SIP_FILTMODE, filterMode, 3, 0);

If we say that the average name string is 8 chars:
Sample instruments: 16 * 18 * 8 = 2304 bytes
Midi instruments: 16 * 5 * 8 = 640 bytes

So if the above if correct, approx 17kB of ram savings to be had with just the above.

The above next need to be validated with actual code changes and measuring change in ram usage.

This all makes sense and I agree. We could implement it as an intermediate step, but I'd take it a step further. Each of those variables use ~150b each IIRC (so about 55k just in variables), and effectively hold a 4byte value. While it does have other functions, I'd say that re-engineering this whole thing would be a very high impact item, specially to be able to add more instruments.

maks commented

Brilliant, thanks @democloid I was hoping you would say that! :-)
I was looking at doing the lowest impact change, but since you are ok with much more invasive change, that definitely will yield the best results! Using Variables and then having Instruments just be containers is a clever design but really inefficient for the pico where ram is so scarce a resource.
My plan is to just have each instrument type use a struct for its data fields, so midi instruments should only be about 12bytes and sample instruments around 50bytes from my quick calculations. If you are ok with this approach? I'll work on a PR.

@maks keep in mind tho that those registered variables are used in the UI code to be able to change their values. So it may not be as easy as it seems. Haven't looked at it in detail tho. Do you have a plan on how to hook those up already?

maks commented

Most of the variables in sample instrument and all in midi aren't watchedvariables, just plain variables, so the pattern in the UI is just eg. v = instrument->FindVariable(SIP_CRUSHVOL); and then the pass it into a intVarField_ and then the intVarField_ just uses the variable ref to draw UI and update it on user input.

But it is a good point that its going to be a very big code diff to change all of this as then the UI will need so now I think about it some more, its probably better to stick to my original plan and reduce size of Variables first and then later move on to removing their use altogether. Looking at Variable.h I think it should be possible to get them down to about 20bytes each just by doing what I said above and also creating a new specific StringVariable subclass.

maks commented

I think this was basically done in #155