[MNG-8575] Replace a list with O(N²) performance by O(N) at least during iteration.

[desruisseaux]

JIRA issue: MNG-8575

The use of AbstractList with an implementation that creates the whole list every time that get(int) is invoked implies that iterations over that list would have a performance cost of O(N²) where N is the number of elements. Extending AbstractSequentialList instead brings back the performance cost to O(N) at least during iterations.

[desruisseaux]

It brings a related question. The following code also has a O(N²) performance cost if a large number N of sources are added through this method:

    private List<SourceRoot> sources = new CopyOnWriteArrayList<>();

    public void addSourceRoot(SourceRoot source) {
        if (!sources.contains(source)) {
            sources.add(source);
        }
    }

We could reduce the cost to O(N) by using LinkedHashSet instead of a list. However, CopyOnWriteArrayList is thread-safe while LinkedHashSet is not. Do we really need thread-safety for that particular field while everything else in MavenProject is not thread-safe? I agree that we may want thread-safety later, but maybe it would need to a more general strategy (e.g. with immutable records)?

[cstamas]

Maybe we want two methods to add sources? And one could be a "batch" (several sources at once) to be used by Project Builder and other adding one by one like for example some plugin may be? Am unsure would the rate of second call justify complicating things....

[desruisseaux]

A batch method would not fundamentally change the O(N²) nature of the operation, unless it temporarily copy the list in an hash set, in which case we lost the concurrency characteristics of CopyOnWriteArrayList.

I think that the first question still: do we need to care about concurrency or thread safety now, given that the rest of the class is not thread-safe? If no, we can just replace CopyOnWriteArrayList by LinkedHashSet and we are done.

[gnodet]

A batch method would not fundamentally change the O(N²) nature of the operation, unless it temporarily copy the list in an hash set, in which case we lost the concurrency characteristics of CopyOnWriteArrayList.

I think that the first question still: do we need to care about concurrency or thread safety now, given that the rest of the class is not thread-safe? If no, we can just replace CopyOnWriteArrayList by LinkedHashSet and we are done.

I don't think we need thread safety here. Thread safe

A batch method would not fundamentally change the O(N²) nature of the operation, unless it temporarily copy the list in an hash set, in which case we lost the concurrency characteristics of CopyOnWriteArrayList.

I think that the first question still: do we need to care about concurrency or thread safety now, given that the rest of the class is not thread-safe? If no, we can just replace CopyOnWriteArrayList by LinkedHashSet and we are done.

I don't think we do. We can defer that at the time we reverse the MavenProject implementation to be implemented on top of Project.