concurrent-trees icon indicating copy to clipboard operation
concurrent-trees copied to clipboard

Reduce compile target to Java 5, address serialization issues and improve performance

Open raphw opened this issue 3 years ago • 3 comments

Thanks for this library, I have not found an equally comprehensive standalone implementation of a radix tree for Java, I really appreciate that you put this out!

I was planning to use this tree for a tool that I develop and I was wondering if you would consider this pull request with the following changed:

  1. Changed compile and source target to Java 5. I need to support several legacy systems with this tool, therefore this would really help me out. It only requires some minor changes in the usage of interfaces and a custom implementation of SetForMap. I added a build plugin to validate that no Java 6+ signatures are used to assure that in-compliant API use would be discovered upon building the project.
  2. For serialization, I added serialVersionUIDs to make sure structural changes can be communicated to the serialization mechanism. Previously, no UID is given.
  3. I removed a few raw types, marked fields as final where possible, inner classes as static and fixed the copyright headers which must be placed as a regular comment and not as a javadoc comment. (one star rather then two). I also changed the build to display warnings for all discouraged code. I also provided an empty array to the array factories which was found to be more JIT friendly (https://shipilev.net/blog/2016/arrays-wisdom-ancients/#_introduction).
  4. I altered explicit checks in the node factories to become assertions rather then explicit checks. In my flight recordings, these checks are rather prominent and be disabling them in production, the overhead can be avoided. At the same time, users that would experience problems can always enable assertions to possibly discover an issue.
  5. I introduced an Automatic-Module-Name to allow using this library within the Java Module system.

Let me know what you think of this and if you wanted me to reduce this request. I hoped I could get my changes upstream to avoid building my own fork but I also understand if you have a different opinion. Looking forward to hearing back! Rafael

raphw avatar Jul 18 '21 05:07 raphw

LOL, Raphel. Just met you this weekend and the first thing to hit me on Monday afternoon when browsing my open tabs is this. Did you publish the fork in the end? This has not seen updates in six years and seems unmaintained.

fatso83 avatar Jun 19 '23 09:06 fatso83

Small world! I have not, unfortunately. But I would certainly consider it. Do you have a need for it?

raphw avatar Jun 19 '23 09:06 raphw

No, I think the current version is sufficient for our needs, but would of course not say no to an actively maintained version, so was just wondering if I could point my pom file to no.winterhalter or something. But no worries, I am good 😄

fatso83 avatar Jun 19 '23 11:06 fatso83