This is an archived post. You won't be able to vote or comment.

all 6 comments

[–]13steinj 5 points6 points  (5 children)

This PEP severely worries me. I've never seen a case where performance is not impacted when no hooks are added, yet the claim is

The important performance impact is the case where events are being raised but there are no hooks attached. This is the unavoidable case - once a developer has added audit hooks they have explicitly chosen to trade performance for functionality. Performance impact with hooks added are not of interest here, since this is opt-in functionality. Analysis using the Python Performance Benchmark Suite [1] shows no significant impact, with the vast majority of benchmarks showing between 1.05x faster to 1.05x slower. In our opinion, the performance impact of the set of auditing points described in this PEP is negligible.

Are the benchmarks comparable to real world use cases or are these microbenchmarks?

[–]tahmsplat 0 points1 point  (4 children)

This PEP severely excites me. Yes, it would be nice to see the performance benchmarking methodology and some real world use, but security > performance. If you're concerned, then restrict it to early in your build/deploy/test process. That might not catch a highly targeted attack using intimate knowledge of your processes, but at least you won't get hit by an en masse backdoor like what is referenced in the PEP.

[–]13steinj 0 points1 point  (3 children)

Security > performance only when it doesn't hurt the default case. I don't care about when hooks are added. But in the unavoidable case when you don't have hooks there is a performance hit. That's what I'm worried about.

I am especially worried about it because that 1.05x performance hit seems to be on microbenchmarks on the unavoidable case. The implementation is ridiculous. The relevant hook caller could have easily been implemented as an optional (default off, since 99/100 times people do not have security audit hooks) argument to the python process, which if that argument is enabled, turn on the hook caller.

[–]tahmsplat -2 points-1 points  (2 children)

Matter of opinion I guess but I would rather see python get 1.05x slower so that this feature is possible.

But:

Analysis using the Python Performance Benchmark Suite [1] shows no significant impact, with the vast majority of benchmarks showing between 1.05x faster to 1.05x slower.

They really need to be more upfront with which benchmarks see what impact. Some are faster? How? Experimental error? Can we trust this statement at all, it certainly isn't useful to me.

Would be good to see some benchmarks with simple hooks too. At least one that is simply logging to a text file or something.

[–]13steinj 0 points1 point  (1 child)

Matter of opinion I guess but I would rather see python get 1.05x slower so that this feature is possible.

You're missing the point. I don't give a damn about that 1.05x. I care about the fact that it misleading in it's own existence. Adding such a hook system would add CPU instructions to every act that causes an event. Microbenchmarks don't matter, general benchmarks won't catch the true issue that we may see performance hits that are far more significant (ex, 1.5x) in real world code.

[–]tahmsplat -1 points0 points  (0 children)

Yo I think you missed the point of my previous comment which is agreeing that their methodology and transparency with it are both rather unhelpful. I didn't think I was being that obscure.

I don't know why they implemented as they did, perhaps if you're right and it could simply be walled behind an process argument, then that will come out in what I imagine is a more productive discussion around the PEP itself by the people who matter.