New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for ByteBuddy as an alternative to CGLIB [SPR-8190] #12840

Open
spring-issuemaster opened this Issue Apr 4, 2011 · 29 comments

Comments

Projects
None yet
1 participant
@spring-issuemaster
Copy link
Collaborator

spring-issuemaster commented Apr 4, 2011

Chris Beams opened SPR-8190 and commented

Tracking issue for CGLIB problems and possible solutions when moving to Javassist or, as recently suggested, Byte Buddy.


Attachments:

Issue Links:

  • #8831 Latest Spring AOP + CGLib caches classes from the same classloader causing CCE in Grails ("depends on")
  • #10325 Consider using javassist for proxy generation ("depends on")
  • #12142 @Configuration with AspectJ instead of CGLIB ("depends on")
  • #7964 Look at phasing out cglib with javaassist ("is duplicated by")

44 votes, 45 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Apr 6, 2012

Kyrill Alyoshin commented

Is it possible to give this issue a bit for weight. CGLIB is one big disaster. It comes with an old ASM version, unmaintained, and buggy. I think Spring is the only project left that is actually using it.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Apr 13, 2012

Chris Beams commented

@Kyrill,

It's definitely on the radar. It would be helpful to hear about exactly what pain this causes you... is it actually ASM version conflicts, for example? What else?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Apr 13, 2012

Kyrill Alyoshin commented

ASM is huge. I have no firm evidence for it, but I also think CGLIB still leaks memory in some scenarios. Fundamentally, CGLIB prevents us from using what you, Chris, have been working so hard on: configuring Spring in Java. :-)

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Jul 10, 2012

Jason Day commented

I have issues with CGLIB leaking memory in OSGi environments where a bundle that uses CGLIB to create proxies will not let them go, even after removing the bundle. I suspect it's related to the root of #8831. The result is that the permgen space eventually runs out after multiple re-deploys of bundles that use CGLIB. Currently, it's merely an inconvenience in our development environments that requires us to occasionally restart our instances since we plan to restart production instances on every application deployment.

To reproduce the issue, create a Spring bundle with a CGLIB enhanced, for example an empty @Configuration file such as the following:

@Configuration
public class MyConfig {
  // class is enhanced by CGLIB
}

Then deploy the bundle to an OSGi environment. Each time the bundle is stopped and started again a new instance of the MyConfig class is re-created, each time enhanced by CGLIB, which gets cached. Take a thread dump and search for "MyConfig" in the heap and you'll see each CGLIB enhanced instance hanging around. This was produced with Spring 3.1.1 and CGLIB 2.2.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Dec 19, 2012

Phil Zoio commented

Get a similar issue when using module redeploys using Impala. Any module which uses the @Transactional where the proxying is class rather than interfaces based is affected. Each time module reloading recurrs for these modules, some more perm gen space is used up, until it eventually runs out.
Ultimately, this issue strongly discourages making any hot redeploy of modules available in production environments, so it would be a real bonus to get it sorted.

Have been using Spring 3.0.6 and Cglib 2.2. Is this problem likely to have been fixed by an upgrade to Spring 3.2, and if not, how much longer should we expect for a resolution?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Dec 19, 2012

Chris Beams commented

Phil, I'd like to make sure this is pinned down once and for all. Can you provide me with reproduction steps to get up and running with the simplest possible impala-based app that demonstrates this issue? While you're at it, please double check against 3.2. We did make some tweaks to how CGLIB caching is done there, but I cannot be sure whether it will solve your problem without looking more closely.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented May 16, 2013

Tim Jones commented

I am not sure how far this issue has progressed but if it helps I have created a few reproducible tests that can be run in Karaf that I think highlight the issues discussed here and in https://jira.springsource.org/browse/SPR-5654

Attached is a zip file that contains the tests and hopefully reasonable instructions on how to set up the tests. I have also added a few observations of my own.

In summary I have set up 2 bundles, with one bundle exporting a transaction manager and the other bundle referencing the transaction manager with a class that has been transactionally advised via a @Transactional annotation.

The tests are further divided with one group dependent on Spring 3.1.4 and the second group dependent on Spring 3.2.2. Within these two groups there are two referencing bundles, one that is coded against interfaces (=> JDK proxy) and the other without interfaces (=> CGLIB).

Both the Spring 3.1.4 and Spring 3.2.2 test bundles that use interfaces seem to be fine, i.e. the referencing bundle can be loaded and unloaded many times without resulting in a OutOfMemoryError: PermGen space error. I was able to verify using JVisualVM that the classes are collected although there seems to be a difference as to when they are collected (details of which are in the readme of the zip).

Both the Spring 3.1.4 and Spring 3.2.2 test bundles that do not use interfaces and hence CGLIB comes into play ultimately fail with a OutOfMemoryError: PermGen space error. I was able to verify using JVisualVM that the classes are not collected and that after uninstalling the referencing bundle that instances of the test classes still remain.

It is possible that the tests could be simplified further, perhaps one bundle rather than two would suffice but I think separating the transaction manager is more realistic.

I realise that class loader permgen leaks are an old well trodden issue that came to the forefront when undeploying web apps but because OSGI permits breaking an application down into fine grained modules I think this issue will become even more apparent as the promise of undeploying/stopping/starting parts of an application becomes a significant driver for the adoption of OSGI.

I hope this helps.

Regards,

Tim

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented May 16, 2013

Tim Jones commented

Tests highlighting perm gen leaks using Karaf

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented May 17, 2013

Chris Beams commented

Tim, thank you for the detailed report. An actual move from CGLIB to Javassist is not high on our priority list right now. Could you create a new Bug, copy your comment over as the description and attach the zip there? That is more likely to get attention in the near term.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented May 19, 2013

Tim Jones commented

I have created a new bug as requested https://jira.springsource.org/browse/SPR-10573

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented May 6, 2014

Rafael Winterhalter commented

Did you ever consider moving to something else then javassist? I want to suggest Byte Buddy (http://bytebuddy.net) as an option which is a rather new library. For full disclosure, note that I am the author of this library but I still hope this posting is not conceived as too self-promotional. I wrote the library after working with cglib and javassist for quite a while and I believe that I took the best ideas out of both frameworks but replaced the bad ideas with something better. I think Spring could profit especially by Byte Buddy's retention of primitive types, the smaller-sized proxy classes and the improved support for implementing around-advice. At last, with Byte Buddy repackaging ASM correctly from day one, users would not have to deal with cglib version conflicts anymore.

Finally, note that the makers of cglib moved their project to GitHub where they however stated that the library is officially not longer under active development. (cglib/cglib#3)

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Sep 22, 2015

Juergen Hoeller commented

We intend to stick with CGLIB for the time being, picking up CGLIB 3.2 whenever it gets released (which still seems to be planned).

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Oct 12, 2016

cemo koc commented

Spock also has just started use ByteBuddy. Mockito is using as well. It might be a good idea to revisit ByteBuddy to replace CGLIB for Spring 5.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Oct 12, 2016

Juergen Hoeller commented

From my perspective, we're happy enough with our repackaged version of CGLIB 3.2.4 against our custom ASM fork at this point, so this isn't a high priority for us in 5.0. I'm sure ByteBuddy is a fine piece but I'm also sure we're going to have some trouble and some regressions, beyond the migration effort to begin with. Keep in mind that we are not just using CGLIB for AOP proxies but also for runtime-generated configuration subclasses and generated lookup methods, for example.

In any case, I've brought this ticket back to life, leaving it in the 5.x backlog for the time being.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Oct 12, 2016

Rafael Winterhalter commented

I am currently helping Hibernate to migrate from Javassist to Byte Buddy after they had some issues and Spring is a big enough framework that I can offer you to try building a prototype.

Since Byte Buddy is just a class generator, it should be easy enough to switch library. In the end, only the generated code is exposed. If you wanted to, you could even mimic the "PoweredByCglib" part of the class name even though I would suggest something more meaningful such as SpringProxy or the like. Let me know if you are interested in my help, porting something from the cglib Enhancer is usually very straight forward!

Also, FYI, I still use some time maintaining cglib but the development has ended. We are only integrating critical bug fixes but we do not plan to apply any bigger extensions for future Java releases.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 19, 2016

Oliver Drotbohm commented

One thing that came up in a discussion recently is that the pure weight of ByteBuddy is quite an obstacle. 2.6 MB is pretty hefty considering that current Spring Core is 1.2 MB and already including inlined CGLib, ASM and Objenesis. Even considering the proxying subsystem as a whole (AOP, Beans, Core) we're still at ~2MB on our own so that ByteBuddy would more than double the package. Did you consider to splitting the library up so that the footprint could be reduced?

What would definitely be interesting to see is — in contrast to strive to replace CGLib right away — whether there's a way we could provide users the option to switch to ByteBuddy if they explicitly opt into it.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 19, 2016

Juergen Hoeller commented

Frankly, from a size perspective, I'd rather handcraft minimal ASM-based class generation code for our own purposes. The end result would probably be in the 200-300 KB ballpark, since we literally just need dynamic subclass generation for non-interface-based AOP proxies, for configuration classes and for lookup method replacements, always just overriding a few methods to intercept straight calls to them. As far as I'm aware, Weld and OpenWebBeans use custom code there as well, primarily for performance reasons (they need to optimize more aggressively due to the overuse of proxies in CDI) but possibly also for jar size reasons.

Then again, we simply don't have any issues with CGLIB at this point, not even on JDK 9 or OSGi. Our approach of generating proxy classes into the application class loader, with references to application code as well as framework code, doesn't mind references to our embedded variant of CGLIB (since such generated code depends on other parts of Spring core anyway). We do not make assumptions about the presence of annotations either: For introspection purposes, we always look at the user-declared class - if necessary, traversing up to the superclass of a CGLIB proxy class - which is quite an established pattern already.

FWIW, we always bind our variant of CGLIB to our embedded variant of ASM. Back in 2013, we made CGLIB 3.0 work against our opened-up version of ASM 4.2 for Java 8 support, so we didn't even have to wait for CGLIB 3.1 in Spring Framework 4.0 milestones. The recent CGLIB 3.2 upgrade was largely a bonus, and our opened-up ASM 5.1 variant works fine on JDK 9 already... Once ASM 6.0 is out, we'll apply the corresponding upgrade to our fork, but we don't have to wait for it, and we in particular don't have to wait for an ASM-6.0-based CGLIB version. It all works fine with our custom variants for our purposes on JDK 9 already.

I'm sure Byte Buddy is a fine piece of engineering, and its swiss army knife capabilities for creating classes from scratch are impressive. However, for our purposes, it looks like overkill: >2 MB extra mandatory jar size for no particular benefit with our use cases, not even for JDK 9 compatibility. Finally, there may be new side effects and maintenance issues: While CGLIB has several stakeholders and is small enough to be patched by ourselves, Byte Buddy is not even GA for a year yet and a huge single-maintainer codebase. It may be more actively developed at this point, but even with the best of intentions, there are no guarantees for the years to come.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 19, 2016

Juergen Hoeller commented

The above said, I certainly don't mind a ByteBuddyAopProxy variant as an alternative to our CglibAopProxy. The question is just when we'd choose it: The plain presence of Byte Buddy on the classpath doesn't seem like a clear enough indication, in particular when brought in through Hibernate or the like.

I wouldn't bother with replacements for configuration class and lookup method processing since we'd have to introduce a specific class processing SPI there for a start. Runtime configuration classes don't get into the hands of user code anyway, and lookup methods are a niche feature.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 19, 2016

Oliver Drotbohm commented

I wonder: are issues like #18894 something tied to CGLib? I.e. would those become easily solvable with something like ByteBuddy in place?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 19, 2016

Juergen Hoeller commented

Well, I guess a custom proxy class could artifically redeclare annotations from interfaces, even in contrast to Java's usual inheritance rules... But effectively, this is simply us not checking corresponding interface methods for annotations in such a scenario. The easiest solution is an explicit lookup on our end there, in a particular with our rather sophisticated merged annotation support.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 20, 2016

Rafael Winterhalter commented

I did consider splitting up Byte Buddy in the past and even did a prototype. In the end, the core dependency would have been something around 1.7 MB with dozens of extension modules which would have made the library much more difficult to use, Another issue with this modularization, with Byte Buddy being a convention over configuration framework components need to know about default implementations and not only some interface and I did not think that class path discovery would be the right approach to overcome this either. Therefore, I decided to retain the single jar file, also because I do not think we would have a different discussion over a 1.7 MB dependency. Many users of Byte Buddy that create Java agents shade and minimize the jar anyways what then creates the minimal sized-jar for the specific use case.

That said, I fully understand that you do not want to bring in a dependency of this size when it even overweights your core dependencies. From my own experience, I would still not recommend you to spin your own proxy mechanism. As a matter of fact, this is where I started out; I needed to write a small replacement for cglib that generated simple proxy classes. This "small library" is now Byte Buddy. Creating proxy code that is performant is not a trivial task and it took me years of fine-tuning to reduce the overhead of the proxy code to a minimum. For example, cglib's MethodProxy class quickly adds so many branches to its invoke method that its creates an inlining barrier for the JIT. On average, cglib proxied-calls bring 10 times the overhead on each invocation compared to Byte Buddy. In the end, this is the reason I suggested you to use the library in the first place; I can very much understand why you chose cglib as it perfectly covers your use-case.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 20, 2016

Juergen Hoeller commented

Rafael, good point about proxy dispatch paths and inlining. An optimized dispatch code path would certainly be worth an option at least, i.e. back to the optional ByteBuddyAopProxy variant suggested above. It'd be great to experiment with that ahead of our 5.0 GA still.

Also, point taken, designing custom class generation code is non-trivial... but not unusual out there, we're even doing it ourselves for our SpEL compilation mode already. It is probably the only way to go for a default footprint as minimal as we have it with CGLIB now.

Ollie, with respect to annotation retaining, here's an interesting one from the Bean Validation world: https://hibernate.atlassian.net/projects/BVAL/issues/BVAL-491 ... If a library/framework is unaware of dynamic subclassing, it may evaluate redeclared annotations as user-specified on subclasses and apply certain constraints to that, assuming an explicitly conceived hierarchy on the user's end.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Dec 16, 2016

Rafael Winterhalter commented

I finally found the time to prototype a solution: raphw@ba838f6 - In the end, it was not difficult at all.

I am not entirely sure about the caching part, but all in all it should work out. I have not yet tested much of the solution but I would love to talk it through with someone. Is there a way to get in touch?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Dec 20, 2016

Rafael Winterhalter commented

I updated my PR with a hacked property-based switch in order to be able to run the unit tests. Everything seems to work just fine. Let me know what you guys think.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Jan 13, 2017

Juergen Hoeller commented

Turning this into a dedicated ByteBuddy ticket, and putting it into our 5.1 backlog for the time being since we're rather time-bound for 5.0 already: 5.0 RC1 is scheduled for end of March, and aside from completing our reactive story, we still got the mess that is JDK 9 to sort out for good.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Jan 13, 2017

Rafael Winterhalter commented

Thanks for the feedback and I fully understand. Just ping me if you need anything.

About Java 9: Byte Buddy only needs an ASM6 update and supports Java 9 from there on, all the internals are already laid out.

As for cglib, I added a patch to support Java 9 for cglib: cglib/cglib@d6fe1d8
The original developer of the library is however difficult to get hold of, therefore, there is no new release yet but since you import, you might want to use the head version anyways.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Feb 13, 2017

Rob Winch commented

I'm not very familiar with ByteBuddy, but this comment makes me worried about passivity and ensuring that Spring Framework's ByteBuddy works with our dependenies' version of ByteBuddy.

As an anecdote, I want to talk about what I had to do just today when updating dependencies on a project using both Mockito and ByteBuddy.

I had Mockito on a recent enough 2.5.4 and a separate internal library had ByteBuddy on 1.6.0. When I upgrade Mockito to its latest as of today, I end up with Mockito on 2.7.5. ByteBuddy dependency from Mockito is at 1.6.5, however. Additionally, ByteBuddy 1.6.5 breaks public API with 1.6.0 by renaming at least one interface and one method (lest you think this is unusual, ByteBuddy has always broken public API in its patch versions; arguably, that's the root cause). Now suddenly, I have two compile-incompatible versions of the same library when I upgrade from Mockito 2.5.4 to 2.7.5.

I'm not bringing this particular example up to assign blame (I love ByteBuddy to pieces, even with its painful unpromise of public API compatibility; I have accepted that it's a high-maintenance library to keep up with) or to suggest changes to Mockito (though, in retrospect, Mockito can shade ByteBuddy as a private dependency to avoid passing down the pain of keeping up with it). I only wanted to point out that maintaining dependencies can be work, even hard work. Predictable, compatible releases are valuable because they make this work easier.

I'm not sure if passivity has already been considered, but I wanted to bring this up to ensure we think about all angles.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Feb 13, 2017

Rafael Winterhalter commented

I understand the concern and it is true that I have changed some APIs in Byte Buddy before the library became so popular. With users like Hibernate and Mockito I am more cautious with changing APIs. Sometimes, it is unfortunately not avoidable as Java byte code - unless Java - is not always backwards compatible itself but I think the APIs are isolated appropriately.

As the user with the problems referred to breaking changes in the recent versions, I can only assume that the mentioned internal library relied on Byte Buddy internal APIs or very bleeding-edge features like module redefinition which some people use to make their software run on Java 9 where the APIs change simply because Java 9 is not yet final.

I do not think this should be a big problem for an optional binding like that one planned for Spring.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Aug 9, 2018

Rafael Winterhalter commented

Just a quick follow-up: if it was possible to refactor the proxy supplier in general, one could consider to generate proxy classes during build time. Byte Buddy's proxy classes are stateless as they are currently implemented such that the runtime could simply locate the classes by their name. These names could for example be added as an annotation value to the proxied classes as a part of the same build operation.

This had the obvious advantage of avoiding class generation during runtime what is expensive. Furthermore, the Byte Buddy dependency would no longer be required during runtime what reduces the footprint of the using application. Finally, this would allow the usage of the proxy mechanism with full AOP capabilities even for native-image generation using Graal or other AOT compilation.

If this was a desired feature, I would happily provide some help creating this implementation. The dynamic proxy generation would however still be helpful, especially during testing where on-demand generation still makes perfect sense.

I created a small POC for this idea: https://github.com/raphw/proxy-example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment