voltace / browser-cookies

Tiny cookies library for the browser
The Unlicense
89 stars 18 forks source link

Move regular expressions out of function body #10

Closed wesleytodd closed 7 years ago

wesleytodd commented 7 years ago

So that every call to set does not have to create a new regular expression object. Should be good for perf.

voltace commented 7 years ago

I agree this would be faster when setting multiple cookies, however do note that the goal of the library is to provide the smallest size possible. To quote the README.md:

The design goal is to provide to smallest possible size (when minified and gzipped) for the given API while remaining compliant to RFC6265 and providing cross-browser compatibility and consistency.

So on the performance-vs-filesize scale this library focuses purely on small file size. It's not set in stone for example major performance issues could be addressed.

It may not be obvious from the source code as most optimization is done by the minifier and gzipping, but the library is actually optimized until the point I wasn't able to reduce the file size by a single byte anymore. Adding variables outside of the function has a significant impact on file size (>10 bytes) so I would prefer to keep the regex inline.

wesleytodd commented 7 years ago

I would argue that a better maxim would be to optimize for "overall performance". But is that is what you want then fine by me, I will just fork. Thats the joy of open source :)

voltace commented 7 years ago

Sure forking is fine! :)

Out of curiosity I googled for regex compilation performance and Chrome caches regex literals (don't know about other browsers) so perhaps there isn't a performance difference after all: http://stackoverflow.com/questions/14352100/does-v8-cache-compiled-regular-expressions-automatically

I used to store the length of an array into a variable before iterating the array, but apparently it's nowadays faster to retrieve the array length at each iteration. Seems a lot of the minor performance tweaks from the old days are not required anymore.

wesleytodd commented 7 years ago

Wow, yeah the optimizations that js engines do are amazing, and often very unexpected. Closing this PR.