Miserlou / Glance-Bookmarklet

A Speed Reading Bookmarklet
https://gun.io/blog/openspritz-a-free-speed-reading-bookmarklet
MIT License
1.56k stars 247 forks source link

String.prototype.repeat - resolves #78 #79

Open Barbarrosa opened 10 years ago

elicwhite commented 10 years ago

Can you link where you pulled the code from? I can't seem to find it with a quick Google search

Barbarrosa commented 10 years ago

This is new code based on the ECMA 6 draft spec: http://people.mozilla.org/~jorendorff/es6-draft.html#sec-string.prototype.repeat

I obtained the meat of the string-concatenating code from this jsperf test: http://jsperf.com/repeat-string-n-times

tomByrer commented 10 years ago

Does this work back to IE8? Have a way to test this out please? That is an interesting jsPerf test either way!

Barbarrosa commented 10 years ago

You can simply include the raw file on a page and load it in IE8. The script running will be the updated version, but it will load the normal Spritz CSS, etc.

Barbarrosa commented 10 years ago

I tested it in Chrome by modifying the bookmarklet.

tomByrer commented 10 years ago

Fast, but let's go faster ^

Barbarrosa commented 10 years ago

The checking against NaN, Infinity, <0, and Math.floor/Math.abs are necessary for the ECMA 6 compliance. That'll still be faster than the current Spritz method, though.

Also, following the spec means we can expect the same behavior (but faster) when a browser implements the method natively (at least with a mature implementation).

tomByrer commented 10 years ago

Cool, so why strict ES6 compliance really needed? Are the results different? If num is always an in - range number, is there a check that needs to run? On Mar 16, 2014 8:03 PM, "Barbarrosa" notifications@github.com wrote:

The checking against NaN, Infinity, <0, and Math.floor/Math.abs are necessary for the ECMA 6 compliance. That'll still be faster than the current Spritz method, though.

Reply to this email directly or view it on GitHubhttps://github.com/Miserlou/OpenSpritz/pull/79#issuecomment-37779670 .

Barbarrosa commented 10 years ago

You may not have received my previous edit in your email. I think this explains it quite well:

"Also, following the spec means we can expect the same behavior (but faster) when a browser implements the method natively (at least with a mature implementation)."

tomByrer commented 10 years ago

we can expect the same behavior (but faster)

I would hope so; there should be alot of redundant mov ops that should be removed, which will also allow the CPU to further optimize the opcodes by interpolation & concurrent execution. (I've done contract work optimizing assembly code by hand.)

But there still ES5 native functions that are faster if you re-write in ES3, like .map, so I will not bet on native being faster. In fact I would bet against it (Chrome 35 result). EDIT: Preliminary results on Firefox 29 Aurora look good, but I almost think they used my version.

If those specific checks are really needed, sure let them be used. But otherwise, why make slow down peoples' browsers & run down their battery more? Also, it is unlikely majority of mobile browsers will have ES6, so I'd prefer my version as a shim.

Barbarrosa commented 10 years ago

It's a tradeoff between performance and human efficiency. People who use the function without seeing the source may assume it works like the ECMA 6 spec.

It may befit this project to use a different function altogether and avoid changing the native String object.

Barbarrosa commented 10 years ago

I would advise against overriding functions that have a spec without also following the spec, lest you reduce interoperability with other JavaScript libraries.

tomByrer commented 10 years ago

use a different function altogether and avoid changing the native String object... that have a spec without also following the spec

Totally agree; I forgot to consider how that function was called. Thanks for pointing that out!

BTW, I think your routine is faster than the current es6-shim. I wish I had time to to a PR for you (funeral & looking for employment). But if you could please make a PR & jsPerf test to prove that your 100% spec version is faster, I'm sure many would benefit from your wisdom.

Barbarrosa commented 10 years ago

It looks like the es6-shim is actually faster, so perhaps we should consider switching to that algorithm regardless of whether we actually use String.prototype.repeat.

JannieP commented 10 years ago

I concur with the es6-shim strategy, especially the "repeat" sub function. var repeat = function(s, times) { if (times < 1) return ''; if (times % 2) return repeat(s, times - 1) + s; var half = repeat(s, times / 2); return half + half; };

However I also agree with [Barbarrosa] that we should avoid overriding functions especially since we are only using the overided repeat for the padding.

why not just call this "repeat" sub function as a standalone function?

tomByrer commented 10 years ago

It looks like the es6-shim is actually faster

Do you have a perf for that please? I could not find a perf to prove that; I looked for one & don't have the time to make it.