ProfileFileSupplier aggregate files in order

[dave-fn]

Motivation and Context

The implementation of aggregate does not guarantee the profile files were submitted to ProfileFile.Aggregator.addFile() in the order given by the suppliers.
The current implementation uses a Map to keep track of whether a supplier returns a different object. Unfortunately, the values() method does not guarantee an ordering.

Modifications

In order to preserve ordering and thread-safety, a LinkedHashMap will be used to feed the aggregator.

Testing

Added two unit tests.

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

Checklist

• I have read the CONTRIBUTING document
• Local run of mvn install succeeds
• My code follows the code style of this project
• My change requires a change to the Javadoc documentation
• I have updated the Javadoc documentation accordingly
• I have added tests to cover my changes
• All new and existing tests passed
• I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.
• My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

• I confirm that this pull request can be released under the Apache 2 license

[dave-fn]

values won't necessarily return the values in order since it's backed by a Map.

[millems]

Could we switch it to a linked hash map?

[dave-fn]

Yes. The thing is we lose the Concurrency from the current map, but we can synchronize manually. Would that be a preferred solution?

[millems]

Should we just wrap currentValuesBySupplier in a Collections.synchronizedMap instead of synchronizing ourselves?

If there's some problem with that:

Can we synchronize just around the writes to the currentValuesBySupplier so that we don't hold the lock while calling the supplier?

ProfileFile current;
synchronized (currentValuesBySupplier) {
    current = currentValuesBySupplier.put(supplier, next);
}

I think we also need to synchronize our iterating over the values on line 150, since modification of the collection during iteration is undefined.

synchronized (currentValuesBySupplier) {
    currentValuesBySupplier.values().forEach(aggregator::addFile);
}

[dave-fn]

I agree the easiest solution is just to wrap into Collections.synchronizedMap.

Yep, I missed the synchronized keyword on line 150.

[sonarqubecloud]

SonarCloud Quality Gate failed.   

4 Bugs
0 Vulnerabilities
0 Security Hotspots
3 Code Smells

91.8% Coverage
1.5% Duplication