On 10/10/2012 02:56 PM, Tim Bunce wrote: > On Wed, Oct 10, 2012 at 09:09:21AM +0200, Steffen Mueller wrote: >> >> [...] around 10-20% faster [...] > > That seems surprising to me, given the change. Even for a micro-benchmark. Same here. I said so before, but other benchmarks showed 1-2%. Callgrind showed 1% fewer instructions executed. >> Of those things, this is left: >> - Code review. >> - Test some CPAN against a SILENT_NO_TAINT_SUPPORT perl. >> - Configure support. >> - Review the remaining taint test failures of the core modules. Some >> fail for a reason because they check -T specifically, some should >> just be skipped if not ${^TAINT} since they test taint compatibility. > > + Gain a deeper understanding of where the costs/savings actually are. Yes, that would be nice. Alas, it's also a project that's 1-2 orders of magnitude more work from where I stand. And it's not just work. It requires me to become smarter. Basically, we struggle A LOT with benchmarking these "small" changes. Trying to disseminate further must therefore come from complete understanding or very detailed profiling/accounting instead of just "poke with a stick and see". I'm not good enough a C programmer or familiar enough with perl's implementation that I could do that. > Accessing an interpreter structure element should be very cheap. > Given the size of the struct perhaps there's some L1 cache thrashing. > In which case perhaps clustering together hot elements may help > reduce the cost of taint checking, and perhaps much else besides. It's a fair number of run-time checks + at least one assignment per NEXTSTATE. > Also, while browsing around I noted that SvTAINTED(sv) is defined as > (SvMAGICAL(sv) && sv_tainted(sv)) and I wonder if that should include > a test for TAINTING_get before (or perhaps after) SvMAGICAL. Sounds like that change alone would fall under the "lost in benchmarking noise" category -- maybe. Admittedly, I won't know without trying as per what I said above. > Perhaps there are unexpected costs elsewhere. Basically I'm saying that > digging deeper here may yield lessons that could be applied elsewhere. Quite possibly, but it'll be for somebody better than me. > + Experiment with more static branch prediction. > > I recall some previous experiments with using branch prediction hint > macros. I think they weren't applied extensively enough to prove a > difference either way. There are only 11 in the current code, most in > utf8.c, and none in headers. > > Wrapping TAINT_get, TAINTING_get and SvTAINTED with UNLIKELY(...) may help: > # define TAINT_get UNLIKELY(PL_tainted) > at least for compilers that support __builtin_expect, i.e. GCC. > http://linuxkernelpanic.blogspot.ie/2010/06/using-static-branch-prediction-with-gcc.html In the implementation of Sereal, I used lots of those and found that there were one or two cases where it was quite obviously helpful and otherwise a wash. On those former cases, it wouldn't have been too hard to reorganize the code to have the same effect based on what appears to be the default assumption. --SteffenThread Previous | Thread Next