Skip to content

Concurrency fix and efficiency improvement at low srate from jfrey-xx#89

Closed
chkothe wants to merge 1 commit into
sccn:masterfrom
chkothe:pr/fix-queue-bug
Closed

Concurrency fix and efficiency improvement at low srate from jfrey-xx#89
chkothe wants to merge 1 commit into
sccn:masterfrom
chkothe:pr/fix-queue-bug

Conversation

@chkothe

@chkothe chkothe commented Oct 30, 2020

Copy link
Copy Markdown
Contributor

This is a merger of #70 along with the queue fix from #71 (and some tweaks).

@tstenner

tstenner commented Nov 2, 2020

Copy link
Copy Markdown
Collaborator

So far, looking good. The locking seems fine to me and the benchmark that exhibited the crash / infinite loop runs through just fine.
Two things: there are a bunch of indentation changes that make it hard to see what changed (or who changed a line last with git blame) and the commit authorship didn't survive the rebasing.

I've split the PR into two commits (@jfrey-xx's changes and yours) and once the CI is happy I'll merge it.

@jfrey-xx

jfrey-xx commented Nov 2, 2020

Copy link
Copy Markdown
Contributor

Thanks a lot for bringing in the changes! It will for sure help a lot here :) Very minor remark, but since I noticed it: in the changelog from tstenner@340d517 I believe that the race condition is actually from #71 ^^

@tstenner

tstenner commented Nov 2, 2020

Copy link
Copy Markdown
Collaborator

Thanks a lot for bringing in the changes! It will for sure help a lot here :) Very minor remark, but since I noticed it: in the changelog from tstenner@340d517 I believe that the race condition is actually from #71 ^^

fixed

@tstenner tstenner closed this Nov 2, 2020
@chkothe

chkothe commented Nov 2, 2020

Copy link
Copy Markdown
Contributor Author

Ah sorry, my editor was still set to indent with 4 spaces instead of tabs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants