Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use WeakMap for caching #51

Closed
wants to merge 2 commits into from
Closed

Use WeakMap for caching #51

wants to merge 2 commits into from

Conversation

jviide
Copy link
Collaborator

@jviide jviide commented Dec 23, 2018

In a Twitter conversation https://twitter.com/webreflection raised the point that template literal statics are cached. They're cached by call site, which makes them ideal for this purpose. Combining this with WeakMaps (which are also supported in IE 11) allows fast caching while avoiding cache retention of unnecessary templates.

This pull request first modifies the benchmarks to use tagged template literals whenever possible to match the most common usage pattern. After that it switches to WeakMap based caching.

The code size goes down, and the "usage" benchmark numbers improve:

Before change

Build "htm" to dist:
        649 B: htm.mjs.gz
        596 B: htm.mjs.br
        714 B: htm.umd.js.gz
        645 B: htm.umd.js.br

console.log test/perf.test.mjs:45
  Creation: 39,626/s, average: 25µs
console.log test/perf.test.mjs:75
  Usage: 243,791/s, average: 4µs

After change

Build "htm" to dist:
        640 B: htm.mjs.gz
        592 B: htm.mjs.br
        706 B: htm.umd.js.gz
        637 B: htm.umd.js.br

console.log test/perf.test.mjs:45
  Creation: 46,867/s, average: 21µs
console.log test/perf.test.mjs:75
  Usage: 363,535/s, average: 2µs

When I ran the benchmarks without Jest the difference in the "usage" benchmark was even more pronounced:

Before change

Usage: 549,356/s, average: 1µs

After change

Usage: 1,542,630/s, average: 0µs

In the thread there was a suggestion to fall back to Map if WeakMaps aren't present. I didn't implement that, because all platforms (with the exception es6-shim) support WeakMap if they support Map.

The drawback in this approach is that the library now requires WeakMap. However, it appears that platforms that don't support WeakMap natively also don't support tagged templates natively.

@WebReflection
Copy link

WebReflection commented Dec 23, 2018

Just 'cause you mentioned the conversation, all this has been tested for over a year through hyperHTML, and these are the things to consider:

  • IE 11 doesn't have template literals, but Babel 7 (and 6) preserve the standard behavior ("slightly" differently due recent specs change)
  • IE < 11 doesn't have WeakMap (neither probably Map) but it has Babel freezing the template, meaning you need to monkey patch Babel upfront or common WeakMap polyfill cannot work due frozen Array, something like this on top of the page would do: this.WeakMap||(function(O,f){f=O.freeze||O;O.freeze=function(o){return'raw'in o?o:f(o)}}(Object));
  • with that, you can bring WeakMaps down to any IE and legacy browser using @ungap/weakmap
  • TypeScript is broken, and so is Firefox < 55, so you may want, in case you are not transpiling via Babel 6/7, fix at runtime the template literal uniqueness. I use @ungap/template-literal for that, which is battle tested

If you ditch the WeakMap module, 'cause nowadays pretty much everyone targets IE 11 as baseline, where WeakMap is already available, you most likely need to address the uniqueness of the template literal issue due bad transpilers such TypeScript (2 versions with two different bugs 'cause they can't get specs right, maybe latter works).

If you think this is worth the performance improvement over standard browsers/transpiled code, you also have the module to include 😉

If you think this project should never ship in production without pre-processing, then maybe you might also ignore the WeakMap/Template improvement, and focus on avoiding CSP issues due code evaluation.

I hope I've properly explained template literals gotchas, since older projects have already dealt with all of these to grant universal compatibility through any client/server engine.

Regards.

P.S. if size is that important, maybe this is a quick win too

function html(statics) {
	return '.' + statics.map(c => [c.length, c]).join('');
}

@developit
Copy link
Owner

Could the produced function be cached as a property set on the statics array?

@WebReflection
Copy link

Nope, 'cause template literals are frozen arrays by specs

@jviide jviide mentioned this pull request Dec 24, 2018
@jviide
Copy link
Collaborator Author

jviide commented Dec 24, 2018

Thanks for the good info @WebReflection. It certainly sounds like experience earned through hard work :) Your thoughts, @developit - is WeakMap caching worth the trouble for HTM?

The CSP point you raised is a very valid one. PR #53 is an attempt to address that.

@jviide
Copy link
Collaborator Author

jviide commented Jan 30, 2019

I think this can be closed for the time being.

@jviide jviide closed this Jan 30, 2019
@jviide jviide deleted the weakmap-cache branch January 30, 2019 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants