Bugfix for trigger matching #92
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chenel
approved these changes
Nov 18, 2024
There was a problem hiding this comment.
Hmm. This seems to be a problem with the trigSeed reference suddenly not referring to anything and having junk memory in it. I can't see how based on the code; trigGroup gets updated, but only at the back of the dequeue, so the front should still be ok. Oh well, if this works empirically and fixes the memory stomping, then let's do it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When 3 trigger streams are used as input, there seem to be a memory issue in the trigger seed.
I assume it's because the seed was updated at each time the
trigGroupwas updated with only two streams compared to now where we can still have the same seed while thetrigGroupis updated.Joined 2 images before and after the change with a
coutto show the trigger seed (or trigGroup.front().second) time + the number of matched triggers.Before the fix, we had some crazy times for the seed and additional 19 triggers due to that issue that is fixed by that commit. (see attached fix_trigger.pdf )