Feature/trim cache head

[interim17]

Time Estimate or Size

small

Problem

This PR no longer addresses the ability to provide a desired max size or length for the cache array, but rather establishes a boolean flag for whether to implement caching at all.

An app like binding sim will be able to pass in false for that prop and the cache will be limited to the single current frame.

Caching according to size is in development.

Solution

Viewport

• Take an optional boolean flag in the viewer props implementCache and propogate to visData

VisData:

• add public method setCachePreferences, to toggle implementCache

• add private method trimCacheHead to trim cache down to single frame

• New feature (non-breaking change which adds functionality)

[github-actions[bot]]

jest coverage report 🧪

Total coverage

Status	Category	Percentage	Covered / Total
🔴	Statements	40.31%	2049/5083
🔴	Branches	42.89%	845/1970
🔴	Functions	36.69%	418/1139
🔴	Lines	40.52%	1962/4841

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

Show files with reduced coverage 🔻

### Reduced coverage | Status | Filename | Statements | Branches | Functions | Lines | | :----: | :------------- | :--------- | :------- | :-------- | :------ | | 🟡 | src/simularium | 59.94% | 57.32% | 48.89% | 60.79% | | 🔴 | VisData.ts | 32.34% | 36.36% | 40.9% | 33.07% | | 🔴 | src/viewport | 14.78% | 10.81% | 15% | 14.33% | | 🔴 | index.tsx | 14.78% | 10.81% | 15% | 14.33% | > Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

[blairlyons]

1001 frames is not a limit, just tends to be a good number for file size and playback time for many of our examples

[meganrm]

LGTM. This seems good as an initial step, though based on your discussion with Dan and Megan it seems like this is likely to be overhauled in the near future? I'll defer to Dan on that one!

Yeah, I'm in favor of no default length, because as Dan says, it's really hard to predict was a real upper limit is. So I think max cache length should only be a setting provided by props, and otherwise not used. And then in the next PR include a max size as we discussed, that is based on browser memory limits (and adjustable by a user prop )

[toloudis]

addFramesToCache uses a ParsedBundle. Basically it's caching the AgentData array. You could write a very simple function that gets the size of an AgentData (which is a fixed size plus variable number of subpoints), and run through the list and add up all the sizes (how many bytes is a number in js? 8? Which brings up the fact that it's probably double what is in the ArrayBuffer representation that came across the network). You could compute the size before the data is moved into the ParsedBundle and the ParsedBundle could carry an extra member variable like cachedSize.

[interim17]

addFramesToCache uses a ParsedBundle. Basically it's caching the AgentData array. You could write a very simple function that gets the size of an AgentData (which is a fixed size plus variable number of subpoints), and run through the list and add up all the sizes (how many bytes is a number in js? 8? Which brings up the fact that it's probably double what is in the ArrayBuffer representation that came across the network). You could compute the size before the data is moved into the ParsedBundle and the ParsedBundle could carry an extra member variable like cachedSize.

Yes you're right. I realized some of that, about parsedBundle being derived from something we know the size of earlier today! I think this makes knowing the overall cache size pretty easy if increment it as we add frames to cache.

The bundle doesn't get stored as a bundle though, the unpacked frames get cached, and bundles carry a variable number of frames, so I'm trying to decide is how to store the relationship between the bundle size and the number of frames it contained so we take the right number of frames off when we trim cache.

First approach I tried today was storing the total cacheSize and then for each bundle also storing bytes and number of frames. So as we trim frames off the cache, we also trim another array that tells us the amount to decrement the overall cache size. There's probably a more elegant way but that's what I have in draft at least.

[interim17]

addFramesToCache uses a ParsedBundle. Basically it's caching the AgentData array. You could write a very simple function that gets the size of an AgentData (which is a fixed size plus variable number of subpoints), and run through the list and add up all the sizes (how many bytes is a number in js? 8? Which brings up the fact that it's probably double what is in the ArrayBuffer representation that came across the network). You could compute the size before the data is moved into the ParsedBundle and the ParsedBundle could carry an extra member variable like cachedSize.

this draft pr https://github.com/simularium/simularium-viewer/pull/384 has a basic implementation. but I don't like storing the extra object cacheFrameSizes it definitely needs work

we could take the bundle size, divide by number of frames, and add cachedSize to FrameData?

This is also not performant, when it kicks in during playback it really slows things down to be trimming the cache and then adding to the cache at the same time.

[interim17]

So in discussion w @toloudis this week we are making progress on caching according to size, but I don't want to rush it.

Really for binding sim the feature we want is: the ability to not implement caching at all, the user provided size is not critical at this point.

So I think it makes more sense to just have a boolean prop implementCache, and when it is false, limit the cache to the single current frame. I tested this in binding-sim app this morning and didn't find any issues.

Later when we implement cache size limits, there can be both the boolean flag, and if true, an optional user provided size, as well as default sizes etc. These would be cachePreferences or something like that.

For now trimCacheHead is hard coded to take cache down to 1 frame as its only called when implementCache is false, but we can make it more flexible when we add other options to cache preferences.

[interim17]

@toloudis @meganrm just so everyone is on the same page: changed the data management bits to enableCache but left the front end prop as disableCache per updated feedback from megan, at this point I'm agnostic :)

[interim17]

@toloudis your suggestion above works with a spread operator so I removed trimCacheHead