aesteve / nubes

Annotation layer on top of Vert.x 3
Apache License 2.0
121 stars 35 forks source link

Suggestion: consider using FastClasspathScanner to find annotations #69

Open lukehutch opened 7 years ago

lukehutch commented 7 years ago

I created FastClasspathScanner for detecting classes and methods with specified Java annotations at runtime.

Nubes currently uses Reflections for its annotation support, which is slower and buggier than FastClasspathScanner (in particular, Reflections is not as actively developed, bug reports can languish for months or years). Reflections also supports fewer classpath specification mechanisms than FastClasspathScanner, and depends on Guice, which is a rather large dependency (FastClasspathScanner has no dependencies).

I am about to start using Nubes in a project, and (though clearly I'm biased) I would love to be able to use Nubes with FastClasspathScanner. In the long run, Nubes would gain a lot of benefits from switching to FastClasspathScanner.

The code changes will be minimal -- you would replace

    Reflections reflections = new Reflections(verticlePackage);
    Set<Class<?>> classes = reflections.getTypesAnnotatedWith(Verticle.class);

with

    ScanResult scanResult = new FastClasspathScanner(verticlePackage).scan();
    List<Class<?>> classes = scanResult.classNamesToClassRefs(
            scanResult.getNamesOfClassesAnnotatedWith(Verticle.class));

etc.

(As with Reflections, you would only need to run the scan once for different class graph queries on the package passed to the constructor -- the ScanResult can be reused.)

Are you at all interested in switching? The main benefits would be increased portability (to a range of classloader environments), speed and robustness.

aesteve commented 7 years ago

tl;dr : Yes ! Absolutely !

Regarding the performances, it shouldn't make that much of a difference, classpath scanning should only be used at server startup and not during runtime (during request/response processing). Especially because it may be blocking and/or involving IOs, but runs in an event-loop. But anyway, any improvement is interesting on this field.

My main concern is, as you mentioned, the amount of dependencies reflections introduces, which increases alot the size of the fatjar... And any improvement on that matter would be much much appreciated !

So yes, I'm very very interested in the change, I've always told myself reflections was way too heavy for what I'm using it for (I'm sure it tackles a lot of other problems though) and always wanted to find a "lightweight" alternative.

Not sure I'll have the time to implement it though (before some weeks) feel free to submit a PR if you want to, or I might give it a shot in the long run.

Thanks a lot Luke for the pointer, that'd definitely something I added in mind for a long time.

lukehutch commented 7 years ago

@aesteve I may not get time to do this in the next few weeks either, but hopefully the changes won't be extensive, whichever of us attempts it :-)

I do think that server startup time matters a lot, because in case you need to restart the server, on a heavily loaded server milliseconds do matter to user experience. (This is partly why I focused so much on performance in developing FastClasspathScanner.)

BTW you may be interested in this feature request I filed in the vertx bug tracker to add route annotation support to vertx-web. Vert.x Nubes is very nice, and I built something similar myself (described in the bug report). https://github.com/vert-x3/issues/issues/237

aesteve commented 7 years ago

I'll try to find some time.


Re. your issue, beware that a lot of people already asked this before and the answer has often been the same : "Vert.x is an un-opinionated toolbox, and should remain un-opinionated of the way you're declaring the routes". Also, vertx-web aims at being polyglot, which defacto excludes the use of annotations (ruby, etc.).

Maybe the position of the official maintainers have evolved, and anyway, it's good you asked, because they're all very open-minded people. But I understand their point of view. The route declaration though lambda is the most generic approach and fits every single use-case, even though a bit verbose sometimes. Letting end-users create some "wrapping, opinionated frameworks" is a good approach.

Also, a lot of people have embraced the same way as Nubes: Qbit, vertx-jersey, etc. There's a ton of projects out there dealing with route declaration. You may be interested in their approach / code, too.

Last but not least, I made a proposal for GSoC 2017 dealing with route declaration. Even though I designed a controller/action framework with Nubes, I like the lambda / method reference approach of pure vertx-web, because it's very flexible. What bothers me the most is having the first lines of code extracting parameters from the routingContext, that is what creates the most verbosity. I like the way Spring MVC, Jersey, and Nubes are dealing with parameters. By reading the method signature, you kinda know what the deal is.

The controller / action stuff comes on top of that, but it's not (imho) what matters the most with Nubes and such frameworks.

That's why I created #70 . I'd love a framework where any @FunctionalInterface could be a proper Handler<RoutingContext> as long as you told Vert.x how to resolve the parameters. The better would be at compile time, so that you have a strong typed safeguard saying "no, you can't have Consumer<Foo> as handler, cause I have no clue how to create a Foo from RoutingContext.

That was really the main goal of Nubes at start : having a very extensible framework letting any end-users create their own resolvers, interceptors, & stuff.

lukehutch commented 7 years ago

That's a lot of great thoughts and perspectives. And thanks, I'll check out the other projects you mentioned.

Yes, Vert.x is intentionally trying to become generic and infinitely flexible, without baking in too much high-level, specialized functionality. But for one of its very common usecases (serving HTTP endpoints), it really could make things easier!

Your GSoC propsal is interesting -- you could do some of that (dynamically instantiating a class with an instance that is able to intercept method dispatch) using the JDK Proxy/InvocationHandler mechanism. Maybe there's a way to use this to address your #70 (for dependency injection etc.)? Although Java doesn't have optional parameters, so I don't know how this would work, other than passing in a null value for the values you want injected, and then marking those parameters up with annotations to overwrite the null with an injected value. I did see this, which you may find useful for lambdas specifically: it looks like JDK and Eclipse recently fixed a bug that allows runtime retention of type annotations on lambda parameters. http://stackoverflow.com/questions/22375891/annotating-the-functional-interface-of-a-lambda-expression

The thing about annotations and language polygotism is that as long as you have a basic constructive API for languages that don't support annotations, then annotations could be considered syntactic sugar. (Also, I don't know much about Ruby, but there are ways to do annotation-like things.)