-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Make some java platform modules optional #2910
Comments
Remember that the benefits of going native imaging isn't so much the size as much as it is the new filesystem JRT, the new class loader, the encapsulated module paths and your 60% on average performance enhancements depending on your personal module structure. That said, the reason why I left most of them as mandatory was to restrict the development to as small as possible with minimal impact. Java.desktop is enormous, the beanutils class is used from there and exists in the top most level databind. Java.logging will be in everywhere, that will always be present in your artefact. My assumption was that size of the deployable would be the smallest concern granted all the other benefits you get by going modular, also when looking at what would usually be included in a module for a program that isn't hello world, it would be really weird to write something without java.sql I'll highlight each use case tonight, and why I left them mandatory, but on an overall perspective, to me a deployable for an ee scale app of 89mb was acceptable. :) |
P.s. I did these implementations during the dev of guicedee, if you want to see total sizes of complete jlink apps try out www.guicedee.com xD |
As far as I remember, these modules are due to pre-existing support for certain types from pre-module era -- all included in JDK by default, so made sense at the time -- and typically cannot just be dropped without consulting user base. With bit of grepping I think:
Of these, I think...
On practical terms, I am not sure if anything can or should be done for Jackson 2.x; users really do not like when things that used to work "suddenly start to fail". It'd probably be against SemVer too. For Jackson 3.0 we could and probably should move them. Maybe there should be a combo package of "jackson-datatype-jdk-extras" or something... included under I'd need help with this, at least for verification, given that I still develop and work on Java 8 platform and have no strong knowledge or actual itch to scratch. |
Additional note: above was wrt actually getting rid of dependencies. Making import "weaker" might make sense for 2.x; I guess what I'd really like to know is just how to handle error cases. There is already dynamic loading for some of the features -- in fact, And second part of course would be that of documenting common error cases. |
@GedMarc my use case is desktop, so smaller image is better for me
If jackson did
I have not checked
Looks like most of the code is ready to be turned in optional. Since jackson does not require these modules transitively, calling code definitely can't pass classes from those modules without explicitly requiring those modules. note: i did not heavily checked anything, i just did some grep and checked code in vim, it took like 15min except writing this comment, it took more |
@XakepSDK thought I would show you this - The base module structure for JDK 9 is this The java.bean package resides in java.desktop, and is used widely by everything - you'll find that i used the bottom levels to provide access - WRT Java 15 and records, the java.bean class already caters for the property reading of these classes, switching the BeanPropertySerializer for something else is a core change, @cowtowncoder will need to say for here (fluent syntax property reading) To do this I believe the modules will need to be defined more strictly for Jackson, I understand the hibernate module is going away, but to do this for you (remove java.sql, not java.desktop), but the Jackson modules will most likely be required to split out to a finer grain, maybe V3?
This is the core jackson class - i do not believe it will be changing any time soon It is quick to check, but a lot longer to check backwards compatibility and effect on support platforms, also to build JLink as there are quite a few funnies - especially with the release of Jakarta Faces 3.0.0 and Jakarta JSON 2.0.0 and JAXB 3.0.0 which i need to make a PR for the jackson-json module now sigh - |
hmm maybe using lombok's & mapstruct's property reader which support fluent and chaining.. but that would add an external dependency |
Oh it's transitive because it's a base JDK library and used by all consumers of Jackson :) it's to make the modular pathway smaller, and not require consumers to define them as requirements :) https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8240567 This is extremely important to use transitive! There is a 64kb limit on the final generated module-info, i would not recommend remove the transitive clause |
It shouldn't be changed at all, this class just weakly uses
|
I believe that:
|
lets give it a try and see - |
there's a few updates for Jakarta compatibility now that they've settled on module names i need to do as well |
Bruh, this whole time i was checking 3.0 branch. Branch 2.12 doesn't look worse. ± same level of work required |
I will test everything tomorrow and probably prepare PR if everything goes well |
@GedMarc Jackson 2.12 has support for Java 15 Records without using any of JDK facilities beyond Java 7. @XakepSDK Sounds good -- it'd be great to get PR soon as I am about to push 2.12.0-rc2 out, and getting this change there would be very useful wrt testing. I am trying to avoid having to do any further rcs before actual 2.12.0 goes out. |
@XakepSDK - JAXB module references java.desktop, looks like I made a note in it - can you have a look? |
I think that usage:
might be relatively simple to replace. And probably was earlier replaced from |
I need some help, more info in PR |
Zero impact on backwards compatibility FasterXML/jackson-databind#2910
Sorry to bring up an old issue, but I'm not seeing the changes for this in the compiled jars of any 2.12 release, even though the source changes are in the branch. Was it reverted? |
@ryanthon I don't think so; I don't recall any reverts. 2.12.5 was released just recently. |
Maybe I'm not looking at the right thing, but doesn't the module-info of the jar need to state "requires static" for the modules described in the OP? I'm not seeing that when I inspect it in IntelliJ. When I use jdeps to list my dependencies, I'm still seeing java.sql and java.desktop in the output. |
@ryanthon Alas, those dependencies are needed due to oddities of JDK handling.
These are quite tricky; support could be dropped/modularized away from databind in 3.0, but unlikely for 2.x. Other than that maybe @GedMarc can help here wrt "requires static". |
Is your feature request related to a problem? Please describe.
Jackson module descriptor implicitly depends on these modules:
requires java.desktop
requires java.logging
requires java.sql
requires java.xml
Can they become optional?
Describe the solution you'd like
Make these modules optional
requires static java.desktop
requires static java.logging
requires static java.sql
requires static java.xml
This may require some code changes, since making module optional means that module may not be present at runtime.
So code should check, if relevant classes are present.
Usage example
jlink and jpackage will produce smaller images.
java.sql
itself brings 10MB+ of dependencies, according to this google/gson#1707 (comment)The text was updated successfully, but these errors were encountered: