search

WebKit / Speedometer

An open source repository for the Speedometer benchmark
Other
592 stars 68 forks source link

Add local storage to todoMVC-es5 #414

Closed flashdesignory closed 3 weeks ago

flashdesignory commented 2 months ago

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

julienw commented 2 months ago

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.

flashdesignory commented 1 month ago

@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.

camillobruni commented 1 month ago

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?

flashdesignory commented 4 weeks ago

changes since last comment: