-
Notifications
You must be signed in to change notification settings - Fork 31
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
Allow use of public fields for getting/setting values #26
Comments
I take this to mean "please allow using fields instead of or in addition to getters and setters", and will modify title appropriately. In future you may consider using titles that reflect feature you want to see, or defect to report. Jackson jr is not specifically designed for Android, although it is one intended target. https://github.com/Instagram/ig-json-parser/ interesting -- it also uses Jackson streaming parser/generator (like jackson jr), but then generates actual data-binding code. |
Hi @cowtowncoder , thx for answering. Sorry, the title was a bit dry and harsh. But still I maintain that IF there is one thing that should be mandatory (for now, using setters/getters), it should be the other way around and it should be fields that are mandatory. Allowing both fields and getters/setters would be nice, but I don't think they offer the same advantages. On Android we need speed, and getters and setters are a lost of cpu cycles and moreover they increase dex methods count which is really a big Android constraint. I don't see how code generation could be something to specific to Android. It is not. It would just allow to completely bypass reflection to find fields, the parsing code could be generated instead of having to build the parsing at runtime. I have spent a lot of time studying Android's annotation processors, reflection, even byte code weaving...etc. And I am fully aware of alternatives like ig-gson-parser or logan Square. I think you should really consider the option to force devs to use non private fields for pojos. That is actually the need of most advanced Android applications. If you want to give your lib a chance, I think that is the option. Again, for speed and dex method count reasons + the opportunity to generate parsing code completely. Also, it might be interesting to have a look at okio/mochi to optimize usages of streams while parsing. I am saying that as I love Jackson, but we are considering to move away from it soon as it doesn't offer the best performances on Android. Your project is really interesting to us but we won't use it if we have to increase the number of methods in our app and/or use reflection still. |
Fair enough. First of all I am not against adding functionality to access fields, if that can be done with relatively modest additions. That actually sounds potentially doable. And that's also how Jackson originally started (Fields were added in 1.1).So if you would be interested in prototyping something for Going backwards from that, jackson-jr specifically, one big goal is to keep package small and this is the main reason for leaving out much of what full Jackson support, including fields. As to code generation, Android bytecode is different from Java bytecode; and so my preferred method of on-the-fly generation differs between JVM and Dalvik. |
You didn't fully understand my point. Sorry, I was not very clear : The main advantage of using fields for Android is immediate : it decreases the dex methods count. Android is limited to 65k methods per app, multidex allows to bypass this limit but there is an overhead and it's slower than not using it. Other topic : code generation vs reflection. Whether you decide to keep setters/getters or fields, you could use an annotation processor to completely bypass using reflection at runtime: you could generate the code for parsing at compile time. It looks like you are talking about generating byte code (that would be specific to a JVM), but that is not what I am talking about. The traditional approach on Android is to generate Java source code, not byte code, and this is fully portable on any JVM. I understand it's a tough path, but from a performance point of view, and also for build compatibility issues, that's the only and best solution around for Android. The point is that without code generation, and with reflection, chances are that all efforts you are making to get a new library working well on Android are lost. There are already a few alternatives around that will perform better, sorry about that. Reflection is really a BIG issue on Android's performances. What about okio ? If you/we (as we are pretty interested to an optimal solution based on Jackson) could achieve to combine :
--> Your lib would be great and a few steps ahead of what is currently available today. There is a tiny niche for it. Otherwise I don't think the lib has a real chance of success on Android. |
Ok. So sounds like there are 2 parts:
I think that at very high level, (2) makes sense. There are 2 projects in fasterxml space that do something related, I think (aside from Afterburner):
Latter could serve as sort of basis; and actually couple of other dataformat modules also use this: CSV, Avro and Protobuf modules all generate internal schema representations to adapt native external format into more JSON like events. At more practical level, it is an open question as to whether this could fit with Having written above, I think the new project idea would make most sense, if anyone wants to pursue that. I do not have time or immediate need to pursue it, but I wouldn't be surprised if others within Jackson community would be very interested. Your points are valid, regarding Android usage, challenges; beyond challenges of using reflection for access, performance of annotation reading is apparently abysmal. I guess engineers didn't think anyone would use them on runtime or something. You could suggest such a project on Jackson discussion groups (jackson-dev at Google groups) to see how others feel. |
Thx for the reply again @cowtowncoder : just a point about your 1) it's really a big thing on Android to decrease dex methods count. Some libs can add up to 30k methods, most of them thousands. This is a real issue on Android. It's really NOT about having to type getters and setters, not at all, but about having an app that fits into a single dex file. Every method you save is good to be saved, and here we are talking about thousands of methods in large app. For a different project specific to Android, well if that happens we would be glad to contribute. Now. But my guess is that's it's going to happen outside of the jackson community sooner. |
Sounds reasonable. I'll keep this as a feature request for (1). The other part is up to whoever has the itch then. |
Implemented; feature must be enabled by:
and with that |
If Jackson JR is designed for Android, it should be using fields. Getters and setters are useless and contribute to reach the dex limit of 65k methods. (Multidex is slow...)
If there was one thing to make mandatory, it would be to use public fields for pojos more than getters and setters.
I know you might be bugged by the problem to access private stuff. But you could also loosen that constraint, if you require the fields to be non-private, you can achieve a super fast lib. You could even create an annotation processor that generates parsing code, which would get read of any/most reflection. Reflection is also a problem on android that should be addressed by such a lib.
The text was updated successfully, but these errors were encountered: