%subj%
Created attachment 132822 [details] Patch
Comment on attachment 132822 [details] Patch Can we test this?
Comment on attachment 132822 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132822&action=review > Source/WebCore/inspector/front-end/HeapSnapshot.js:646 > + return this.className.substr(0, length) === detachedDOMTree; I'd rather use indexOf(detachedDOMTree) === 0
Committed r111392: <http://trac.webkit.org/changeset/111392>
(In reply to comment #3) > (From update of attachment 132822 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=132822&action=review > > > Source/WebCore/inspector/front-end/HeapSnapshot.js:646 > > + return this.className.substr(0, length) === detachedDOMTree; > > I'd rather use indexOf(detachedDOMTree) === 0 Why? That does a search for the string, not a prefix check. Is indexOf == 0 a good design pattern? Maybe the JavaScript engines special-case it?
Comment on attachment 132822 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132822&action=review >>> Source/WebCore/inspector/front-end/HeapSnapshot.js:646 >>> + return this.className.substr(0, length) === detachedDOMTree; >> >> I'd rather use indexOf(detachedDOMTree) === 0 > > Why? That does a search for the string, not a prefix check. > > Is indexOf == 0 a good design pattern? Maybe the JavaScript engines special-case it? indexOf === 0 is just a common way for string prefix check in the inspector front-end code. Only heap profiler uses this substr approach. My point was that we should be consistent here. As for the performance, they should be equally fast given that we don't check for a prefix in strings much longer than the prefix itself.
Looks like we have no special-case for this. I've made an investigation and got the next results. aString = "012345678900123456789001234567890012345678900123456789001234567890012345678900123456789001234567890012345678900123456789001234567890" function test() { for (var j = 0; j < 1000000; ++j) doCompare(aString); } // time: V8:641ms, JSC:799 // produces garbage function doCompare(name) { return aString.substr(0, 6) === "Window"; } // time: V8:643ms, JSC:799ms // produces garbage function doCompare(name) { return aString.slice(0, 6) === "Window"; } // time: V8:928ms, JSC:866ms // no garbage function doCompare(name) { return aString.indexOf("Window") === 0; } // time: V8:565ms, JSC:890ms // no garbage function doCompare(name) { const t = "Window"; const l = t.length; for (var i = 0; i < l; ++i) if (name[i] !== t[i]) return false; return true; }
looks like I have to change this code to RegExp version http://jsperf.com/careful-with-that-indexof-eugene