asdf
asdf copied to clipboard
perf: speed improvements
Summary
This is a draft for now. It pulls in the patch I linked here, as well as some other work. I mostly wanted to get people's opinion on it early before I continue with the entire project.
In general, it speeds up performance by reducing fork calls, either by combining various combinations of sed | grep | cut etc. into a single awk call, or where possible, using shell built-ins like parameter substitution and modification.
Fixes
It addresses but does not fix issue 290
Other Information
I haven't ran the full test suite yet, but I know at least one (where) fails; I believe this to be a false positive, and will address it.
@stephanGarland A few thoughts after a quick glance.
Before we introduce changes like these I would like to standardize and have in our CI pipelines performance testing and reporting to PRs. That way we identify the actual performance uplift of a specific change. (https://github.com/sharkdp/hyperfine is my current candidate for this automation. How to integrate it is something I will look at very soon).
Once we have the above, we can then go through and add each of these changes one at a time. That way we should have clear data about the improvement for each case.
Further, #1433 & #1138 are partially addressing #1396 & #1397 which will lead us down the path towards using more native shell features. I am not sure if changes for those will clash with what you have here, but be prepared for some rework here.
This draft PR is a great place for us to discuss your performance improvements and ideas, so please keep this open, keep updating it, and I will hopefully have time to review the rest of it soon.
Thanks! This is exciting work!
Awesome! Performance increases are a much needed fix.
I know #1433 was mentioned, and as its author, I know that PR is pretty big, but I only fixed formatting (mostly), for only files in test/*, so there should be little to no conflicts. Same with the other PR.
I've identified some Bash incompatibilities, and some possible improvements. There might be some disagreement of some sort about the first suggestion, so maybe that one can be hashed out once the mentioned additional perf test is integrated in the CI pipeline
Before we introduce changes like these I would like to standardize and have in our CI pipelines performance testing and reporting to PRs.
For the record I agree with @jthegedus on this. I'd love to improve performance but I also don't want to be reviewing changes that only improve performance by a small percentage, or changes that only improve things for a small subset of users (not saying that is the case here, I'm just now looking at these changes).
Once I finish my other PRs I will work on the performance automation for this repo.
I found and fixed a bug I had introduced with parse_asdf_version_file that was causing a ton of failures. There are still test failures (34/352, but tbf some of those are from fish and elvish simply because I don't have them installed yet), but the number is greatly reduced.
Happy 1 year anniversary for this PR! Sic.
Happy 1 year anniversary for this PR! Sic.
I got frustrated trying to fix bugs only present in esoteric shells like elvish, and around the same time, shifted to mise (née rtx). Sorry. I might pick it up again once my professional life has calmed down some.