-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Misc perf improvements #1772
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
Misc perf improvements #1772
Changes from 5 commits
032cf7c
c02204f
b4a2f4d
b9c33c9
ea883e6
11f4a0e
a511bd2
434dc0e
d077e32
3132624
00c7ad5
2da3544
dc4fc32
0129d5a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
'use strict'; | ||
|
||
/* | ||
* timeit: tool for performance testing | ||
* f: function to be tested | ||
* n: number of timing runs | ||
* nchunk: optional number of repetitions per timing run - useful if | ||
* the function is very fast. Note though that if arg is a function | ||
* it will not be re-evaluated within the chunk, only before each chunk. | ||
* arg: optional argument to the function. Can be a function itself | ||
* to provide a changing input to f | ||
*/ | ||
window.timeit = function(f, n, nchunk, arg) { | ||
var times = new Array(n); | ||
var totalTime = 0; | ||
var _arg; | ||
var t0, t1, dt; | ||
|
||
for(var i = 0; i < n; i++) { | ||
if(typeof arg === 'function') _arg = arg(); | ||
else _arg = arg; | ||
|
||
if(nchunk) { | ||
t0 = performance.now(); | ||
for(var j = 0; j < nchunk; j++) { f(_arg); } | ||
t1 = performance.now(); | ||
dt = (t1 - t0) / nchunk; | ||
} | ||
else { | ||
t0 = performance.now(); | ||
f(_arg); | ||
t1 = performance.now(); | ||
dt = t1 - t0; | ||
} | ||
|
||
times[i] = dt; | ||
totalTime += dt; | ||
} | ||
|
||
var first = (times[0]).toFixed(4); | ||
var last = (times[n - 1]).toFixed(4); | ||
times.sort(); | ||
var min = (times[0]).toFixed(4); | ||
var max = (times[n - 1]).toFixed(4); | ||
var median = (times[Math.ceil(n / 2)]).toFixed(4); | ||
var mean = (totalTime / n).toFixed(4); | ||
console.log((f.name || 'function') + ' timing (ms) - min: ' + min + | ||
' max: ' + max + | ||
' median: ' + median + | ||
' mean: ' + mean + | ||
' first: ' + first + | ||
' last: ' + last | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -392,15 +392,14 @@ drawing.singlePointStyle = function(d, sel, trace, markerScale, lineScale, gd) { | |
|
||
}; | ||
|
||
drawing.pointStyle = function(s, trace) { | ||
drawing.pointStyle = function(s, trace, gd) { | ||
if(!s.size()) return; | ||
|
||
// allow array marker and marker line colors to be | ||
// scaled by given max and min to colorscales | ||
var marker = trace.marker; | ||
var markerScale = drawing.tryColorscale(marker, ''); | ||
var lineScale = drawing.tryColorscale(marker, 'line'); | ||
var gd = Lib.getPlotDiv(s.node()); | ||
|
||
s.each(function(d) { | ||
drawing.singlePointStyle(d, d3.select(this), trace, markerScale, lineScale, gd); | ||
|
@@ -423,7 +422,7 @@ drawing.tryColorscale = function(marker, prefix) { | |
// draw text at points | ||
var TEXTOFFSETSIGN = {start: 1, end: -1, middle: 0, bottom: 1, top: -1}, | ||
LINEEXPAND = 1.3; | ||
drawing.textPointStyle = function(s, trace) { | ||
drawing.textPointStyle = function(s, trace, gd) { | ||
s.each(function(d) { | ||
var p = d3.select(this), | ||
text = d.tx || trace.text; | ||
|
@@ -454,7 +453,7 @@ drawing.textPointStyle = function(s, trace) { | |
d.tc || trace.textfont.color) | ||
.attr('text-anchor', h) | ||
.text(text) | ||
.call(svgTextUtils.convertToTspans); | ||
.call(svgTextUtils.convertToTspans, gd); | ||
var pgroup = d3.select(this.parentNode), | ||
tspans = p.selectAll('tspan.line'), | ||
numLines = ((tspans[0].length || 1) - 1) * LINEEXPAND + 1, | ||
|
@@ -611,19 +610,18 @@ drawing.makeTester = function() { | |
// in a reference frame where it isn't translated and its anchor | ||
// point is at (0,0) | ||
// always returns a copy of the bbox, so the caller can modify it safely | ||
var savedBBoxes = []; | ||
var savedBBoxes = {}; | ||
var savedBBoxesCount = 0; | ||
var maxSavedBBoxes = 10000; | ||
|
||
drawing.bBox = function(node) { | ||
// cache elements we've already measured so we don't have to | ||
// remeasure the same thing many times | ||
var saveNum = node.attributes['data-bb']; | ||
if(saveNum && saveNum.value) { | ||
return Lib.extendFlat({}, savedBBoxes[saveNum.value]); | ||
} | ||
var hash = nodeHash(node); | ||
var out = savedBBoxes[hash]; | ||
if(out) return Lib.extendFlat({}, out); | ||
|
||
var tester3 = drawing.tester; | ||
var tester = tester3.node(); | ||
var tester = drawing.tester.node(); | ||
|
||
// copy the node to test into the tester | ||
var testNode = node.cloneNode(true); | ||
|
@@ -655,18 +653,41 @@ drawing.bBox = function(node) { | |
// make sure we don't have too many saved boxes, | ||
// or a long session could overload on memory | ||
// by saving boxes for long-gone elements | ||
if(savedBBoxes.length >= maxSavedBBoxes) { | ||
d3.selectAll('[data-bb]').attr('data-bb', null); | ||
savedBBoxes = []; | ||
if(savedBBoxesCount >= maxSavedBBoxes) { | ||
savedBBoxes = {}; | ||
maxSavedBBoxes = 0; | ||
} | ||
|
||
// cache this bbox | ||
node.setAttribute('data-bb', savedBBoxes.length); | ||
savedBBoxes.push(bb); | ||
savedBBoxes[hash] = bb; | ||
savedBBoxesCount++; | ||
|
||
return Lib.extendFlat({}, bb); | ||
}; | ||
|
||
// capture everything about a node (at least in our usage) that | ||
// impacts its bounding box, given that bBox clears x, y, and transform | ||
// TODO: is this really everything? Is it worth taking only parts of style, | ||
// so we can share across more changes (like colors)? I guess we can't strip | ||
// colors and stuff from inside innerHTML so maybe not worth bothering outside. | ||
// TODO # 2: this can be long, so could take a lot of memory, do we want to | ||
// hash it? But that can be slow... | ||
// extracting this string from a typical element takes ~3 microsec, where | ||
// doing a simple hash ala https://stackoverflow.com/questions/7616461 | ||
// adds ~15 microsec (nearly all of this is spent in charCodeAt) | ||
// function hash(s) { | ||
// var h = 0; | ||
// for (var i = 0; i < s.length; i++) { | ||
// h = (((h << 5) - h) + s.charCodeAt(i)) | 0; // codePointAt? | ||
// } | ||
// return h; | ||
// } | ||
function nodeHash(node) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting patch. Do you have any benchmark results comparing old and new If you're concerned about memory usage, you might want to consider setting cc'ing @monfera @rreusser who might have experience doing stuff like this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't have any explicit benchmarks, but putting in a few
Yes, perhaps I could drop the limit a bit, but two things to note:
|
||
return node.innerHTML + | ||
node.getAttribute('text-anchor') + | ||
node.getAttribute('style'); | ||
} | ||
|
||
/* | ||
* make a robust clipPath url from a local id | ||
* note! We'd better not be exporting from a page | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @rreusser would be nice to have something like that in https://github.com/rreusser/plotly-mock-viewer
Moreover, @dfcreative's brilliant ✨ https://www.npmjs.com/package/fps-indicator ✨ would be a nice addition to our dev tools.