Only inspect one frame in depends decorator #57062
Conversation
Some very good background can be found on StackOverflow: * https://stackoverflow.com/questions/17407119/python-inspect-stack-is-slow I filed an issue about slow `inspect.stack()` in Python some time ago: * https://bugs.python.org/issue39643 Slow `inspect.stack()` is a problem, but it's not the problem in this case. The problem is that Salt asks Python to get a full blown stack with all the frames, but then discards everything but the second frame from the top. This is very wasteful, and it's much more economic to ask for the current frame and walk back once. Here's how long it takes to run `test.ping` with existing code: ``` $ time sudo salt-call --local test.ping local: True real 0m12.166s user 0m11.679s sys 0m0.499s ``` And this is how much better it gets with the proposed change: ``` $ time sudo salt-call --local test.ping local: True real 0m2.092s user 0m1.772s sys 0m0.336s ``` If you follow the advice from saltstack#48773 and disable `vmware` module, which is responsible for the most calls into inspect module, you won't get much better than that: ``` $ cat /etc/salt/minion disable_grains: - esxi disable_modules: - vsphere $ time sudo salt-call --local test.ping local: True real 0m2.006s user 0m1.671s sys 0m0.353s ``` Closes saltstack#48773.
ghost
requested review from
|
The inspect call was introduced by @jacksontj in #5549 Any chances to get this reviewed/merged before Sodium? |
|
@dwoz looks like tests/integration/files/file/base/_modules/runtests_decorators.py are the tests for this depends decorator. |
|
@gtmanfred does that 👎 mean those aren't the tests? |
|
@waynew As a big proponent of testing, I'm sure you know that Having said that, integration tests run the actual Salt code and check that the results of that code match what was expected. Since this fix involves no functional change in outcome, but rather a change to what functions are called within the decorator, I'm not sure I see the utility in adding an integration test for this. |
saltstack/salt#48773 is meanwhile closed, there was an additional feature added to salt [1] and we face problems with our mine (which is mostly based on grains). So lets try to remove this option and see if our mine behaves better after that. [1] saltstack/salt#57062
Some very good background can be found on StackOverflow:
I filed an issue about slow
inspect.stack()in Python some time ago:Slow
inspect.stack()is a problem, but it's not the problem in this case. The problem is that Salt asks Python to get a full blown stack with all the frames, but then discards everything but the second frame from the top. This is very wasteful, and it's much more economic to ask for the current frame and walk back once.Here's how long it takes to run
test.pingwith existing code:And this is how much better it gets with the proposed change:
If you follow the advice from #48773 and disable
vmwaremodule, which is responsible for the most calls into inspect module, you won't get much better than that:Closes #48773.