Use concurrent sets or queues

[jelovirt]

Description

Use concurrent sets and queues where applicable.

Motivation and Context

Contains operations are faster on a dedicated set implementation.

How Has This Been Tested?

Tests pass.

Type of Changes

• New feature (non-breaking change which adds functionality)

Documentation and Compatibility

None needed

[raducoravu]

There is still probably a performance problem with waitList.contains() as it’s still a queue, can’t we use some kind of linkedhashset instead and always remove first item from it?

[jelovirt]

Using waitList = new ConcurrentLinkedQueue<>(); may not be a good idea after all.

The waitList doesn't have to be a queue, but we need some way of removing and getting some value out of the set. We could just use LinkedHashSet, but it's just a Set, so there's no remove() or pop() method.

[raducoravu]

On my side after replacing Queue with LinkedHashSet I used something like:

	while(! waitList.isEmpty()) {
		Reference ref = waitList.iterator().next();
		waitList.remove(ref);
		processFile(ref);
	}

I could not iterate over the waitList and remove at the same time because "processFile" influenced the set while I was iterating through it.

[jelovirt]

Maybe we could use ConcurrentSkipListMap 🤔

[raducoravu]

@jelovirt I'm not sure how many automatic tests for performance has the DITA OT, in this case such a test could be created to populate the sets in GenMapAndTopicListModule with lots of data and then to use the access methods and see how fast they return.. then you could switch implementations inside the class and see if you notice any improvement.

[raducoravu]

Looks ok, I will try next week the changes with the large project.

[jelovirt]

@raducoravu What do you think about this? Are you able to run the same benchmark again?

[raducoravu]

@jelovirt I will run the processing on Monday and report back.

[raducoravu]

@jelovirt I can confirm the latest fix no longer produces the CPU usage problems I noticed with the original code in GenMapAndTopicListModule. I profiled both with the fix (and the problem was no longer noticed) and without the fix (and the problem with the queue.contains() was noticed in the profiler)