search

queryverse / VegaLite.jl

Julia bindings to Vega-Lite
Other
267 stars 39 forks source link

Reduced the time it takes to save a VLSpec as SVG, PNG, PDF #400

Closed rben01 closed 2 years ago

rben01 commented 3 years ago

Rewrote the convert_vl_to_* functions that call the chains of node.js commands that turns a VLSpec into some kind of output (SVG/PNG/PDF), reducing the time that show takes, speeding up save by a factor of around 7-8x for small plots and 60x for large plots on my machine. (Not sure what I actually fixed by rewriting it; I used an IOBuffer to store the input of the pipeline.)

rben01 commented 3 years ago

Similar changes should also be made to Vega.jl. See https://github.com/queryverse/VegaLite.jl/issues/331#issuecomment-854323701 for more info.

davidanthoff commented 2 years ago

Sorry that it took me so long to look at this.

I don't understand this PR, though :) It seems to assume that it is dealing with ProcessChain, but that is actually not the case? And I also don't understand how this could possibly speed up anything? All it does is move one if clause around?

rben01 commented 2 years ago

Sorry, some git snafus (I didn't realize that trying to merge from upstream would also update this PR). I'd intended for the PR to only consist of the commits through 583195251553dc478885b58f38725059e7d25090 (June 5, 2021) -- back when it was still spelled vegaliate_app_path 😉. I've reverted this repo back to the state it was when I submitted the PR, but as a new commit. Not sure what the right way to undo this mistake is -- all I know is 583195251553dc478885b58f38725059e7d25090 is the commit I meant to submit as a PR.

davidanthoff commented 2 years ago

Hm, there still seems to be a git problem? Lots of files edited that probably shouldn't be touched at all?

rben01 commented 2 years ago

I think I may just close this and open another with the fix reapplied. The changes I made don't really apply anymore anyway since the code has changed so much in the meantime.

rben01 commented 2 years ago

Anyway the important changes to buffering and async happen in the first two commits -- everything else can safely be ignored. (The first commit also does a small bit of refactoring to reduce code duplication)

rben01 commented 2 years ago

Opened a new PR for this change