Speedometer icon indicating copy to clipboard operation
Speedometer copied to clipboard

Add local storage to todoMVC-es5

Open flashdesignory opened this issue 1 year ago • 3 comments

This pr switches the storage from in-memory to localStorage for todomvc-es5.

Scores snapshot:

browser before after
chrome 30.5 30.3
firefox 26.12 25.6
safari 25.5 25.3

@kara

flashdesignory avatar Jun 18 '24 20:06 flashdesignory

Hey Thorsten, can you remind me when/where we've discussed about introducing localStorage into a workload?

As far as I know, the latest decision about that was https://github.com/WebKit/Speedometer/pull/251#discussion_r1237232511, that is localStorage introduces I/O in the workloads and therefore is another source of noise, so we decided to not include it in workloads.

It's totally possible I forgot about a past discussion too.

julienw avatar Jun 28 '24 09:06 julienw

@julienw - there was previous concern and we did opt out of using localStorage for our workloads for 3.0.

The reason I think that it's a valid addition:

  1. it's widely used in real world applications and therefore browsers should be able to optimize for the api usage
  2. although we need to be careful with not persisting localStorage between tests, this is absolutely do-able and should not be a reason to avoid it.

I'm also happy to include this in an experimental branch, if there's still concern. Although I believe that we are missing out on opportunities to optimize for our users, if we're not taking storage apis in consideration. They ARE widely used, if we optimize for them or not.

flashdesignory avatar Jul 11 '24 18:07 flashdesignory

Let's maybe incubate this as an experimental workload and see how it behaves on various devices (I have to test this on mobile as well which are known to have slow flash storage).

Maybe we could prime the local storage (aka. a single access in prepare) so we don't necessarily measure the first-time setup costs if they're disproportional to the runtime costs?

camillobruni avatar Jul 15 '24 13:07 camillobruni

changes since last comment:

  • added a dedicated experimental folder in the root of speedometer (this replaces 'tentative')
  • updated benchmark-runner to not prepend 'resources', since the experimental folder is in the root
  • 'experimental' tag replaces 'tentative' tag
  • local storage example accesses localStorage once in the prepare phase (added a helper function to expose localStorage to the Page class)

flashdesignory avatar Aug 06 '24 19:08 flashdesignory