metrics-spring icon indicating copy to clipboard operation
metrics-spring copied to clipboard

AdvisingBeanPostProcessor can call excessively AopUtils.canApply which can lead to performance issue when using Spring prototypes

Open matlach opened this issue 8 years ago • 16 comments

Our application heavily rely on the following pattern:

    @Component @Scope(BeanDefinition.SCOPE_PROTOTYPE)
    public class Prototype { ... }

    @Inject
    private Provider<Prototype> prototypeProvider;

    public Prototype getPrototype() {
        return prototypeProvider.get();
    }

each time we instantiate the Prototype object, the AdvisingBeanPostProcessor is getting called and eventually test wheter we can apply AOP on that prototype class using AopUtils.canApply(pointcut, targetClass)

The implementation of AopUtils.canApply is really heavy weight, it has to drill down in each available methods of the target class which can then lead to performance issue.

I do not know if there would be any ways to optimize the pattern? My initial thought is just to create a Map to store wheter if for a given pointcut + targetClass if the check has already been done. WDYT? Finally, if you think this is the only way, I can provide a pull request with a potential fix.

Thanks,

matlach avatar Jul 19 '16 15:07 matlach

Oh interesting, I didn't realize it hit prototype scoped beans so hard. My only concern is the memory usage of that cache. If only Java had a builtin LFU cache. I've pushed a possible fix: https://github.com/ryantenney/metrics-spring/tree/can-apply-cache. Would you be able to test it and see if there's an improvement?

ryantenney avatar Jul 19 '16 16:07 ryantenney

I should be able to test soon if this resolved our issue. I'll keep you updated but I'm very confident with your proposed fix.

matlach avatar Jul 25 '16 15:07 matlach

We performed a stress test yesterday with your proposed fix an the issue is definitly gone.

matlach avatar Jul 26 '16 14:07 matlach

Thanks so much!

ryantenney avatar Jul 26 '16 14:07 ryantenney

Hi @ryantenney , I was just wondering, do you know when this fix will become mainstream in the 3.1.x branch?

Thanks!

matlach avatar Aug 05 '16 16:08 matlach

Hi @ryantenney , well… I have basically the same question as @matlach: are you planning to release a v3.1.4 with that fix so we can simply update our maven dependencies? Would be awesome! Thanks a lot!

efasel avatar Nov 10 '16 10:11 efasel

Hi @ryantenney , any update on this? Thanks!

efasel avatar Jun 19 '18 14:06 efasel

@efasel I'm sorry I haven't gotten this fix out! I'll cut a release this week.

ryantenney avatar Jun 19 '18 15:06 ryantenney

Hi @ryantenney ! :-)

Two month ago you wrote:

I'm sorry I haven't gotten this fix out! I'll cut a release this week.

Obviously something has come up, but maybe you'll have time soon? Would be great to have a release including this important fix. Thanks!

Best regards, Elmar

efasel avatar Aug 29 '18 10:08 efasel

That was 2 months ago? I'm sorry for keeping you waiting. My personal laptop died and took with it the key I need to sign the release. I'll find a backup and get a release out soon.

ryantenney avatar Aug 29 '18 14:08 ryantenney

Hi @ryantenney, sorry to hear that your laptop died. I hope you didn't lose anything important. Any plan to do this release we're talking about soon? :-) Best regards, Elmar

efasel avatar Oct 01 '18 08:10 efasel

Hi @ryantenney, what about a nice Christmas present for the community and have a v3.1.4 release? :christmas_tree: :gift: ? :smiley: Best regards, Elmar

efasel avatar Dec 06 '18 14:12 efasel

I just got my laptop back from Apple yesterday ;) coming right up

ryantenney avatar Dec 06 '18 14:12 ryantenney

:tada:

efasel avatar Dec 06 '18 21:12 efasel

Hi @ryantenney ! I just saw that you created a tag for v3.1.4 but this was not released to the usual maven repositories. So – as far as I know – we cannot simply update our dependency:

[WARNING] The POM for com.ryantenney.metrics:metrics-spring:jar:3.1.4 is missing, no dependency information available

Are you planning on pushing v3.1.4 as an official release?

efasel avatar Jan 18 '19 10:01 efasel

Hi @ryantenney! How are you today? Just wanted to ask regarding the 3.1.4 release … :-)

efasel avatar Mar 21 '19 12:03 efasel