Skip to content

Conversation

@raphw
Copy link

@raphw raphw commented Jul 18, 2021

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 added 10 commits July 23, 2021 08:48
…lue where from a ConcurrentRadixTree.

By avoiding this allocation, looking up a large chunk of values can be done with less GC overhead. Escape analysis on the JVM is unfortunately rather limited such that this allocation is not properly avoided by the JIT in many cases. If this allocation ends up on the hot path, its collection can have a visible impact on throughput.
…r char-index and length resolution.

Ideally, the allocations are removed by escape analysis but the mechanism does not always work as expected such that search can cause significant allocation.
Allocations can trigger significant garbage collection if not erased by escape analysis.
…decendent nodes.

Unfortunately, the AbstractReferenceArray API and List API are incompatible due to the conflicting signatures of set(int,Object) where one returns a boolean and another declares void. To avoid this conflict, an API is introduced that implements the minimal required functionality but allows a single object to represent any interaction with child nodes. This way, non-leaf nodes do no longer require an additional field to carry the list wrapper, reducing the memory consumption of a tree with many non-leaf nodes.
@fatso83
Copy link

fatso83 commented Jun 19, 2023

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.

@raphw
Copy link
Author

raphw commented Jun 19, 2023

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

@fatso83
Copy link

fatso83 commented Jun 19, 2023

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 😄

@knutwannheden
Copy link

knutwannheden commented Nov 26, 2024

FWIW, for a narrow use case I was looking for an efficient adaptive radix tree (ART) implementation in Java and ended up rolling my own: https://github.com/openrewrite/rewrite/blob/8bd3573ee495863d320d1a5511391b63b1e76886/rewrite-core/src/main/java/org/openrewrite/internal/AdaptiveRadixTree.java. it isn't thread-safe and doesn't support deletes (as I didn't need that), but that can be added. It has zero dependencies, is all in a single compilation unit, and offers fairly good insert and lookup performance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants