ToolboxHub / ToolboxToolbox

Declarative dependency management for Matlab.
The Unlicense
27 stars 11 forks source link

speedup tbParsePrefs (persistent prefs) #102

Closed githubkusi closed 4 years ago

githubkusi commented 4 years ago

The slow command getpref('ToolboxToolbox') is on a performance-critical path. Therefore a significant performance gain (54% in my particular case) can be gained by injecting the result of getpref() to each function

While this PRQ is not too complex, it is quite invasive as it has to add a new parameter for many functions. I did not yet update the tests. I'm happy to do so, but please let me know first what you think of it.

I successfully use this change already for many months.

DavidBrainard commented 4 years ago

Will try to review this weekend and either merge or provide comments. No objection in principle to the idea.

DavidBrainard commented 4 years ago

OK. I merged this, but as you note all the tests now fail. Please update those so they all work and we'll call this good.

DavidBrainard commented 4 years ago

Have you had a chance to work on the tests? It's important to me that these pass.

githubkusi commented 4 years ago

I'm sorry for being slow on fixing the tests. Currently I don't find the time to work on it, but hopefully it still happens this year. I actually just wanted your opinion of this speedup prior to merging :) I would have fixed the tests on the PR before merging, that was the idea. If you feel uncomfortable, feel free to restore the previous state and I do a new PR, this time with tests.

githubkusi commented 4 years ago

actually the tests already fail before this PRQ (8243a8088fbfceaa841296b835ad296bcf4ca5b1) e.g TbGitAndSanityTest/testLifecycle