Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Now message-box/bt class will restart thread if it was aborted because of a non-local exit from processing loop. #103

Merged
merged 3 commits into from
Jan 30, 2025

Conversation

svetlyak40wt
Copy link
Contributor

I've measured performance before these changes and after them and didn't notice significant changes. Results of the benchmark along with the baseline results are available in this gist:

https://gist.github.com/svetlyak40wt/a1097ab7d501087ca366d5addb410ebe

Note, this PR is the replacement for #101

It uses another approach for checking if thread is died – catches non-local exit using unwind-protect.
Also, in this thread I've removed a special brach for processing the time-out while waiting for response. Now both cases (with time-out and without) are handled using the same code, relying on timeout argument of condition wait function.

Peformance testing didn't show any difference with the code from the master branch.

…e of a non-local exit from processing loop.

I've measured performance before these changes and after them and didn't notice significant changes.
Results of the benchmark along with the baseline results are available in this gist:

https://gist.github.com/svetlyak40wt/a1097ab7d501087ca366d5addb410ebe
tests/test-utils.lisp Show resolved Hide resolved
sento.asd Outdated
)))
(:file "test-utils")
(:file "message-box-test"
:depends-on ("test-utils")))))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since test-utils is defined before if the depends-on necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, unless you have :serial t option in the defsystem form.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, just add a serial t then. In main system we have it as well. I think it's straight forward when the loading mirrors the dependency structure roughly.

@mdbergmann mdbergmann merged commit 0c88caa into mdbergmann:master Jan 30, 2025
1 check passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants