Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Compute indexed properties when building the list of PropertyNames #1314

Merged
merged 1 commit into from
Feb 13, 2025

Conversation

radcortez
Copy link
Member

@radcortez radcortez commented Feb 13, 2025

@radcortez radcortez merged commit c279362 into smallrye:main Feb 13, 2025
6 checks passed
@github-actions github-actions bot added this to the 3.11.3 milestone Feb 13, 2025
Comment on lines -244 to +234
Collections.sort(indexedProperties);
return indexedProperties;
Map<Integer, String> indexedProperties = configSources.getPropertyNames().indexed().get(property);
return indexedProperties == null ? Collections.emptyList() : indexedProperties.values().stream().toList();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With autoboxing, I doubt that a map is going to be better than a sorted list with all of the redundant object creation. Also, I think your method of converting to a stream is suspect. Are you certain this will give you your items in index order? Your test shows values in lexicographical order so it might be working accidentally.

Maybe the original problem is the actual sort though; if you're sorting numbers lexicographically, it's likely going to be a problem. It should not be too hard to define a numerically-sorting comparator that is efficient (find first non-zero character in each, compare lengths, then compare Character.digit(x) for each digit from most significant to least).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The underlying map is backed by a TreeMap, and the index is being extracted, converted, and used as a key in the map (basically to discard invalid indexes).

The indexed structure is Map<String, Map<Integer, String>>, where the first key is the unindexed name, and then the TreeMap contains the indexes, with the target property. This was to avoid having to reconstruct the key.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3.10+ - Yaml lists are out of order
2 participants