-
Notifications
You must be signed in to change notification settings - Fork 3k
(De)Serialize ApplicationModel (from) to JSON instead of Java Object Serialization #51430
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
base: main
Are you sure you want to change the base?
Conversation
|
Thanks for your pull request! Your pull request does not follow our editorial rules. Could you have a look?
This message is automatically generated by a bot. |
|
Did you measure the deserialization speed? Because it might also be useful for the |
Yes, both are significantly faster. |
|
Looking at the native tests failure, it looks like i need to make sure I properly deserialize the config. The original implementation did escape character during serialization but didn't unescape them during deserialization. |
|
Should we add some tests for the JSON layer? I'm pretty sure that's something Claude could do for us. |
|
We must add tests. I was a bit surprised there weren't any. |
|
I'll try to see tomorrow if Claude can generate a reasonable set of tests. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributed by Claude
Contributed by Claude
|
@aloubyansky I pushed a fix and some tests contributed by Claude: they seem to make sense and be good base for the future. Let me know what you think, we can adjust or even drop the commit if you're not convinced. |
|
Great! Thanks! |
This comment has been minimized.
This comment has been minimized.
Status for workflow
|
This PR is meant to supersede #50602
It includes the following changes:
quarkus-bootstrap-jsonmodule that includes a slightly enhanced version ofio.quarkus.builderJSON API fromquarkus-builderand makes previous users of thequarkus-builderAPI use the new API.io.quarkus.builder[.json], the new API is in packageio.quarkus.bootstrap.json, since it's in the bootstrap project.JsonObjectBuilderimplementingMapandJsonArrayBuilderimplementingCollectionand unescaping characters when deserializing (it looks like it was overlooked in the original impl).io.quarkus.builderJSON API (should it be simply removed?).io.quarkus.bootstrap.model.Mappableinterface, that basically defines a methodMap<String, Object> asMap(), that allows to represent an application model as aMap.Mappableinterface also allows to passMappableCollectionFactorytoasMap()to create instances of custom implementations ofMapandCollection.ApplicationModelSerializerutility class to serialize and deserializeApplicationModelwith simple methods.quarkus.bootstrap.application-model.serialization.format=jos.LazySourceDir(used in the Gradle integration) replacedDefaultSourceDir(it's easier to deal with it when serializing and deserializing).This serialization approach is apparently significantly more efficient compared to Java Serialization - ~2-5 times faster depending on the application model size.
The file a model is serialized to still has
.datextension. It should probably change to.jsonif the JSON format is used.