Skip to content
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

fix: Maintain insertion of types in a package #4832

Conversation

slarse
Copy link
Collaborator

@slarse slarse commented Aug 8, 2022

Fix #4827

Adjusts the ElementNameMap to maintain an insertion order of it's entries, which in turn allows us to maintain insertion order of contained types. This will result in a performance hit on entrySet() due to the sorting, but I don't think it should be all that bad.

If we want to improve performance further we'll need to implement a concurrent version of a LinkedHashMap, or something similar.

@I-Al-Istannen Thoughts?


// This can not be a "Map" as this class is Serializable and therefore Sorald wants this field
// to be serializable as well (Rule 1948).
// It doesn't seem smart enough to realize it is final and only assigned to a Serializable Map
// in the constructor.
private final ConcurrentSkipListMap<String, T> map;
private final ConcurrentHashMap<String, InsertOrderWrapper<T>> map;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The hash map generally performs better. The ordering of the skip list map wasn't what we wanted.

Copy link
Collaborator

@I-Al-Istannen I-Al-Istannen left a comment

Choose a reason for hiding this comment

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

Looks good to me, maybe a single nitpick

@slarse slarse changed the title fix: Maintain insertion of types in a package review: fix: Maintain insertion of types in a package Aug 10, 2022
@MartinWitt MartinWitt changed the title review: fix: Maintain insertion of types in a package fix: Maintain insertion of types in a package Aug 15, 2022
@MartinWitt MartinWitt merged commit 1ec339d into INRIA:master Aug 15, 2022
@MartinWitt
Copy link
Collaborator

Thanks, @slarse

@slarse slarse deleted the issue/4827-maintain-insertion-order-in-eleementnamemap branch August 15, 2022 21:50
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.

[Bug]: reverse order for classes in source file that declares multiple classes
3 participants