Performance considerations: store only data as value

[itsuryev]

Right now in localStorage we have basket-<key> as key and JSON.stringify({ data: <script>, url: "...", expire: 5, etc: "..." }) as value.
That as result means that we have to parse that object each time we want to check expiration time, etc. Sounds right. The problem here is in the fact that we also store scripts itself with the object. That as result means that stringified version of object could be really large and JSON.parse will not be fast.

In my case I've tried to store 3mb script file.

    basket.requrie({ url: 'really.large.js', unique: 'v1' });

This file will expire only with the next application version. But what happens under the hood each time we load app?
We are calling basket.clear when basket is initialized which gets the item (parsing it), checks expire date and lets it live. Then we are calling require which once again calls get so we get item and parse it again, this time to use data.

The first parse is totally unnecessary in my case and the second could be simpler - we just need data of the script, without any processing.

So what I am suggesting?

I see following options:

1. Store all props data in the key: basket-<key>-<JSON.strigify(props)> and value would be just data.

This way we always will be parsing small object with props and data will stay verbatim.

The downside - key just got complicated and then user invokes basket.get('some.key'); we have to do more to find the element (iterate over all keys finding the one right). Some map can be used - specified key -> actual key.

1. Store data in several entries: basket--data, basket--props. This way it is clear what we need to fetch to get our data but remove and logic which clears older entries have to be modified - we always need to set two keys and always remove two of them.

I am building specific prototype for my app and sort of (minus the map) implemented the first approach.

Please share your thoughts on this matter. If this sounds reasonable and useful we can discuss details and I could implement the solution.

[sindresorhus]

I think both options sounds reasonable, but I think I prefer 1.

Sidenote, we need to ensure that key in basket-<key>-<JSON.strigify(props)> does not contain the separator character -. Let's just use \t for separating the parts.

@addyosmani thoughts?

[addyosmani]

I vote for 1.

[itsuryev]

Sidenote, we need to ensure that key in basket--<JSON.strigify(props)> does not contain the separator character -.

Why?
Please note that we can not change the first basket- part as then remove won't be able to identify old scripts and clear them if they are expired.

Maybe right away you have some ideas on how to deal with key - fullKey map?
If we are ok to not support use case where user manually adds basket entry to the localStorage when I can suggest following approach:

1. Generate map during remove as it either way iterates through all localStorage
2. Updating single entry in the map once new scripts are fetched.
   I believe this is fine as some version(s) ago you have removed add functionality.

My next step would be to send pull request so we would have something to look at. Though I doubt I would be able to do it during this week.

[sindresorhus]

Why?

basket-foo-bar-baz-{}

Parsing out the key foo-bar-baz is totally possible, but unnecessary complex.

If we are ok to not support use case where user manually adds basket entry to the localStorage

We are.

Generate map during remove as it either way iterates through all localStorage

Can you clarify? I'm not sure I see the original issue prompting this.

[itsuryev]

basket-foo-bar-baz-{}
We can write it like this: basket-foo-bar-baz\t{}
This way we have: <basketKey><userKey><separator><JSON.stringify(props)>

<basketKey> = basket-
<userKey> = foo-bar-baz
<separator> = \t

Can you clarify? I'm not sure I see the original issue prompting this.

Sure. When we store everything except data itself in the key the difficult part would be fetching the item we need.
User write basket.get('my-item');. How do we find it in the localStorage?
It will be stored as:
basket-my-item\t{url:'...'}
So we have to iterate over localStore items, try to split based on basketKey, then split based by separator and then check what left between them. If it matches original request - all is good, else - continue iterating. And that part with splitting, splitting again and then matching strings would not be fast itself. So we want to limit it.
What I am suggesting is to form map between user keys and full prefixed keys with props during remove command. During remove we either way iterate though all the collection and try to find all the basket items to check their expiration date. Just as well we can form following map:

var map = {
    'my-item': 'basket-my-item\t{url: "..."}'
};

[sindresorhus]

You're assuming iteration is slow, but I don't really that's the case.

How do we find it in the localStorage?

We do a regex for matching the key: new RegExp('basket-' + key). Then iterate and check.

[itsuryev]

Iteration itself is ok, splitting, splitting and checking is not so.

We do a regex for matching the key: new RegExp('basket-' + key). Then iterate and check.

Indeed, we can even make it new RegExp('basket-' + key + propsSeparator) so we would not receive extra results (e.g. key was 'foo-bar', we might have got 'basket-foo-bar' and 'basket-foo-bar-baz').

Sounds like a plan.

This seems faster than my splitting option.
Still we will have to check how fast/slow it is in real life. Imagine you have 5 items not related to basket and 5 basket scripts in localStorage.
You load your application and as usual require 5 scripts. For each of them we would iterate over localStorage, check regex. That would be 50 checks total. In case of map there might have been just 10 (though have to try first).

[sindresorhus]

Yup, sounds good to me.