Skip to content

MS-930 Implement stateful face matcher that reduces amount of allocations#1151

Merged
luhmirin-s merged 1 commit into
mainfrom
spike/MS-930-reduce-template-allocation
Mar 26, 2025
Merged

MS-930 Implement stateful face matcher that reduces amount of allocations#1151
luhmirin-s merged 1 commit into
mainfrom
spike/MS-930-reduce-template-allocation

Conversation

@luhmirin-s
Copy link
Copy Markdown
Contributor

@luhmirin-s luhmirin-s commented Mar 25, 2025

JIRA ticket
Will be released in: 2025.2.0

This is an experimental implementation of the suggestion in the JIRA ticket, it could be polished to be production-ready if required.

Notable changes

  • Changing the face matches to pre-allocate native sample template memory and reuse it for each candidate. In theory, it should reduce memory and GC pressure.
  • Still using a separate allocation per coroutine to avoid cross-coroutine memory sharing and potential concurrency issues. There is still a significant reduction in allocation calls.

Surprisingly the change is way less noticeable in my very basic benchmarking (running identification on 50k candidates using ROCv3 on a Lenovo Tab 8):

Before change After change
114780ms 92866ms
113032ms 110177ms
107323ms 110511ms
110483ms 95675ms
111383ms 104624ms
113065ms 109125ms
112612ms 110679ms
Avg: 111811 Avg: 104808

As part of this task, I also investigated the feasibility of the same refactoring for fingerprints and decided against it for the following reasons:

  • SimAfis matching supports 1:N on the library level and takes the candidate list as a parameter, therefore it is highly likely that the same allocation optimisation is happening under the hood (or at least it should be done on the library level).
  • NEC wraps templates with a custom data class and there is no way to control allocations. Therefore even if we cache the template list, it would not prevent unnecessary allocations.
  • Fingerprint BioSDK setup is significantly more complex and would require a lot of changes for no/negligible benefit.

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)

@cla-bot cla-bot Bot added the ... label Mar 25, 2025
@luhmirin-s luhmirin-s force-pushed the spike/MS-930-reduce-template-allocation branch 2 times, most recently from 8e88e4b to d2aefc6 Compare March 25, 2025 13:23
@luhmirin-s luhmirin-s marked this pull request as ready for review March 25, 2025 13:36
@luhmirin-s luhmirin-s requested review from a team, BurningAXE, TristramN, alex-vt, alexandr-simprints, meladRaouf and ybourgery and removed request for a team March 25, 2025 13:37
@luhmirin-s luhmirin-s force-pushed the spike/MS-930-reduce-template-allocation branch from d2aefc6 to fdcffc2 Compare March 26, 2025 10:12
@sonarqubecloud
Copy link
Copy Markdown

@BurningAXE BurningAXE self-requested a review March 26, 2025 12:48
@luhmirin-s luhmirin-s merged commit 812ad6c into main Mar 26, 2025
@luhmirin-s luhmirin-s deleted the spike/MS-930-reduce-template-allocation branch March 26, 2025 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants