Optimizations and refactoring for 2.20 #28
Conversation
When a reusable "CORE" sub is leaked, compile and cache its trampoline under the "Fatal" package rather than the original package[1]. This allows autodie to trivially reuse the trampoline on leaks from other packages as well. [1] Note that the wrapper sub is compiled into "Fatal" as well when the "CORE" sub is reusable. Signed-off-by: Niels Thykier <niels@thykier.net>
Call $class->_install_subs exactly once instead of once per sub being
{,un}imported.
Signed-off-by: Niels Thykier <niels@thykier.net>
Signed-off-by: Niels Thykier <niels@thykier.net>
Remove one level of the cache and replace the keys in the "deepest" level with named constants (selected based on $lexical and $void). Signed-off-by: Niels Thykier <niels@thykier.net>
Signed-off-by: Niels Thykier <niels@thykier.net>
Previously, when nesting autodie with itself, autodie would wrap its own wrapper. With this patch, autodie properly detects it is dealing with a "wrapped CORE sub" and simply generates a normal CORE wrapper. Besides removing one level of indirection, it also removes the overhead of checking for hints for the CORE sub (which was somewhat counterintuitive, since autodie normally ignores hints for CORE subs). Signed-off-by: Niels Thykier <niels@thykier.net>
The use of local on this variables have no apparent visible effect at all (except hiding $! for callers). We retain it for Fatal, where at least the modified value of $" is still used by the wrapper. Signed-off-by: Niels Thykier <niels@thykier.net>
|
Ping? |
|
I absolutely want to review and merge all of your changes! However I'm giving the closing keynote for OSCON in an hour! I'm a little low on sleep and cognitive resources as a result. Can you ping me in a couple of days when I'm recovered? You rock so hard. Thank you! <3 |
_expand_tags is not really suited for expanding ":all" due to how it was constructed. However, it turns out that it is quite trivial to expand ":all"-case is a reasonably more efficient manner. Since ":all" is now always expanded, we can abuse this to reuse the value in %tag_cache (_expand_tag). Signed-off-by: Niels Thykier <niels@thykier.net>
|
I will re-ping you in couple of days then. I hope you enjoy/enjoyed OSCON. :) |
Signed-off-by: Niels Thykier <niels@thykier.net>
Signed-off-by: Niels Thykier <niels@thykier.net>
As a side-effect, we can avoid two hash-look ups on every leaky call on CORE subs (after the trampoline has been generated). Signed-off-by: Niels Thykier <niels@thykier.net>
|
Here we go, re-ping. :) |
Refactor import and _make_fatal so they "use less 'CPU';". In
particular, many calls to prototype("CORE::sub") has been replaced
with a cache, which turns out to be cheaper in the long run.
Combined these changes appear to reduce the runtime of benchmark.pl by
about 1/8 (~4.0s to ~3.5s).
Signed-off-by: Niels Thykier <niels@thykier.net>
Commit 9ab0959 missed the "CORE::" prefix in the for the "all"-tag. Signed-off-by: Niels Thykier <niels@thykier.net>
|
Hey, been almost a week since my last ping now. Are there parts of the commits that are unclear to you or something I could clarify to make it easier for you to review my changes? |
|
Niels, you are fabulous, and thank you so much for your patience. I'm looking through these right now. :) On the matter of timely merges, would you like to be a co-maintainer of autodie? I think your knowledge of the internals now surpasses my own, and you most definitely pass the requirement of being completely awesome. :) Paul |
|
Merging now. I'm going to have to run almost immediately after, but I really like these changes! <3 |
Optimizations and refactoring for 2.20
|
Tests pass under 5.8.9, 5.14.2 and 5.16.0. :) |
|
Awesome :) |
|
Also, Travis is drunk again. ExtUtils::Makaker ? I have no idea where that's coming from. |
|
On 2013-08-04 21:46, Paul Fenwick wrote:
I have to admit I am intrigued by your proposal. If I can leave the ~Niels |
|
@nthykier : Awesome! I'm happy to do releases, they're easy. Although if I get hit by a bus, or I otherwise drop off the face of the earth (eg: urgent bugfixes required while I'm at burning man) you may be the new release manager. :) Do you have a PAUSE ID I can associate with the module? |
|
On 2013-08-04 23:29, Paul Fenwick wrote:
Nope, I do not have a PAUSE ID. ~Niels |
|
On 2013-08-04 23:29, Paul Fenwick wrote:
Was that a "please sign up a PAUSE account at " or what happens ~Niels |
|
Back from Burning Man! Yes, that was indeed a "please sign up with a PAUSE account" at pause.cpan.org. Even if you never use the account, it will allow me to flag you as a co-maintainer (and if I get hit by a bus, or a critical change needs to be released while I'm camping in the desert, you can make it.) Paul (hopping on a flight to Australia in 24 hrs) |
|
On 2013-09-05 03:08, Paul Fenwick wrote:
Hey, Just got my pause account, NTHYKIER. :) ~Niels |
I got another 5 patches, which combined cuts the "use autodie" time by another factor 4 (on top of the factor 2 improvement we got in 2.18) on the benchmark.pl case. The major speed improvement is obtained by making autodie lazy (commit ed7c623), so consumers will now only pay for what they use.