Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Eliminate use o ThreadLocal in chained profiles #85

Closed
splatch opened this issue May 10, 2024 · 0 comments · Fixed by #87
Closed

Eliminate use o ThreadLocal in chained profiles #85

splatch opened this issue May 10, 2024 · 0 comments · Fixed by #87
Assignees

Comments

@splatch
Copy link
Contributor

splatch commented May 10, 2024

The chained profile concept outlined in opensmarthouse/opensmarthouse-core#35 which was supplied in this repository have been implemented in 2021 and quickly redesigned in 2022. First and redesigned implementation embedded ThreadLocal to pass invocation which initially worked well.
However, this design have a large drawback of retaining invocation state within a shared object. The main point of it was really to separate creation and invocation time, which could not be shared.

Looking closer at the issue of sharing state, it actually can be bridged between creation and invocation time without ThreadLocal and major multi threading risks. When invocation chain is initialized, chain elements are initialized sequentially. It means that they know own position and can reference chain. In order to simplify invocation, we can also swap use of iterator and rely on chain element index manipulation (+1 from handler to item, -1 from item to handler), in order to avoid direct references.

While I know it sounds very complex, actual change (despite of its size) will result in much simpler code providing the same functionality.

@splatch splatch self-assigned this May 10, 2024
@splatch splatch changed the title Elimite use o ThreadLocal in chained profiles Eliminate use o ThreadLocal in chained profiles May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

1 participant