[MS-949] Improve concurrency in matching

[BurningAXE]

JIRA ticket
Will be released in: 2025.2.0

Notable changes

This PR builds upon 1169 by making the following changes:

• Ranges are created in such a way so that their count is multiples of the CPU cores. This way all ranges can be processed in parallel at all times, minimizing CPU idling and maximizing efficiency. Fore example for a CPU with 4 cores, totalCount will be divided into 4, 8, 12, etc. ranges depending on the size of totalCount. A maximum size of 2000 is imposed for range size.
• MatchResultSet was refactored to handle concurrency as the previous implementation lead to crashes if modified concurrently
• The loadedCandidates counter was also modified to be thread-safe as previously it had inconsistent behaviour
• flows in the face/fingerprint matcher use cases are started on Dispatchers.IO instead of the caller's Dispatchers.Main

Testing guidance

I've tested this extensively with CoSync but would like someone to double check it with the internal DB. Trying out different totalCount's would also be nice to ensure no edge cases are left out.

Additional work checklist

• Effect on other features and security has been considered
• Design document marked as "In development" (if applicable)
• External (Gitbook) and internal (Confluence) Documentation is up to date (or ticket created)
• Test cases in Testiny are up to date (or ticket created)
• Other teams notified about the changes (if applicable)

[Copilot]

Pull Request Overview

This PR enhances parallel matching by introducing more concurrent data structures and improving range generation for batch processing. Key changes include refining the range creation algorithm, updating match result storage to be thread-safe, and modifying use cases to track loaded candidates safely.

• Swapped TreeSet for ConcurrentSkipListSet and used an AtomicReference in MatchResultSet for thread safety.
• Replaced non-thread-safe counters with AtomicInteger in matcher use cases and updated range creation to distribute work evenly.
• Expanded and updated tests for CreateRangesUseCase to cover various edge cases.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
CreateRangesUseCase.kt	Redesigned batch range logic to account for processor count and max size
MatchResultSet.kt	Switched to ConcurrentSkipListSet and atomic reference for lowest confidence
FingerprintMatcherUseCase.kt	Replaced shared var with AtomicInteger for loaded candidates
FaceMatcherUseCase.kt	Applied same AtomicInteger pattern for concurrency
CreateRangesUseCaseTest.kt	Added comprehensive tests for new range logic

Comments suppressed due to low confidence (2)

feature/matcher/src/main/java/com/simprints/matcher/usecases/FingerprintMatcherUseCase.kt:94

• The log message interpolates the AtomicInteger directly, which will print its object representation instead of the count value; use loadedCandidates.get() to retrieve the actual count.

Simber.i("Matched $loadedCandidates candidates", tag = crashReportTag)

feature/matcher/src/main/java/com/simprints/matcher/usecases/CreateRangesUseCase.kt:13

• [nitpick] The KDoc for the invoke function does not document the availableProcessors parameter or describe its behavior; consider updating the comment to include this parameter and its purpose.

operator fun invoke(

[meladRaouf]

I don't think we need .flowOn(dispatcherIO), or at most we should use .flowOn(dispatcherBG). All read operations already run on the IO dispatcher using withContext(dispatcherIO), and all matching logic is executed within launch(dispatcherBG) { ... }.

[BurningAXE]

All read operations already run on the IO dispatcher using withContext(dispatcherIO)

That's the problem - they don't! Reading was happening on the main thread because it used the scope passed from the ViewModel! This is why the UI wasn't updating counts until all reading was done.

[meladRaouf]

I think the flow should run on a background dispatcher, with only the database read operations running on the I/O dispatcher.

[BurningAXE]

That would be the official recommendation. However, in my testing IO provided better performance. I have no explanation why but I don't see any harm, either 🤷

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
96.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[luhmirin-s]

Are atomic reference for "lowestConfidence" and concurrent collection required if the whole block is synchronised?

[BurningAXE]

For how we use it currently - no. But they should not lead to any further slowdown, either.

[luhmirin-s]

Likely splitting hairs, but concurrent data structures are typically much slower.

[BurningAXE]

I'm tempted to dismiss this concern for structures with max 10 results. However, if you feel strongly about this, we can do a quick benchmark and verify TreeSet indeed works fine in real concurrent situations (CoSync reading with N threads)!?