Better trace ids#595
Merged
Merged
Conversation
Member
Author
|
The affected benchmark is this one just in case |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release-6 #595 +/- ##
============================================
Coverage ? 80.76%
============================================
Files ? 212
Lines ? 6466
Branches ? 0
============================================
Hits ? 5222
Misses ? 1244
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Member
Author
|
@fonsp could you adjust |
Member
Author
bvdmitri
added a commit
that referenced
this pull request
Apr 17, 2026
* add Changelog * Implement event handler system for message passing and callbacks (#587) * feat: add event handler system for message passing procedure Introduce a new event handler mechanism that allows users to hook into the message passing procedure via `Event`, `handle_event`, and `broadcast_event` functions. Events are broadcast before and after message rule calls, enabling debugging and monitoring capabilities. Key changes: - Add `event_handler.jl` with event types and handler interfaces - Extend `MessageMapping` to include an event handler parameter - Support both custom handler types and NamedTuple-based handlers - Add documentation for the events system - Include comprehensive tests for event handler functionality * type in docstring * inject the event handler in the factor node activation options * make format * document the existing events better * rename event handler to callbacks to better match RxInfer * update docs * use make format * fix typo * allow to merge callbacks * allow reduce the result of the callbacks * add per-event callback reducer * make format * remove test method * start reimplement of the product of two messages this is required to inject the callbacks properly, plus we do the breaking release already. Could break (and improve) more things as well then * fix initial integration with RxInfer.jl * fix Aqua tests * caught a small bug in RxInfer tests * caught another bug from RxInfer.jl tests * merge stricter formatting * Refactor the variables, add docstrings, add labels - `compute_product_of_messages` now accepts the `AbstractVariable`, makes it easier to identify the variable inside the callback * fix documentation build * 2prev * add before/after product of two messages callbacks * fix failing tests * add before/after product of messages callbacks. add before/after form constraints * add before/after marginal compute callbacks * add error hint in case of wrong passed callbacks * fix test * temporary fix for the logscale switch rule * support Dict as callback handler * fix warnings in the tests * make format * refactor to use an abstract Event{E} structure * add new method for event_name * use mutable fields instead * update CHANGELOG * Trace ID to link before and after events (#589) * Trace ID to link before and after events * Add allocations test * disable some allocation tests on old julia versions * Change sqrt to + and remove version check * only check on julia 1.12 again * Better trace ids (#595) * create IDs only if callbacks are set * fix one failing test * make format * Refactor addon system to annotations (#596) * save plan * update * refine the plan * refine plan 2 * implement post_product/post_rule annotations infrastructure * reimplement the memory addon, remove old implementation * commit new file * make format * update plan * refactor the message.jl and message_tests.jl * update marginals.jl and marginal_tests.jl * remove old bridge functions * refactor the rule macro and the rule generating expression * refactor the nodes and tests * fix remaining places where addons were used * fix a lot of tests by hand * fix more tests by hand * update the plan * finish the refactor, update the documentation with migration guide, update CHANGELOG * fix from RxInfer * fix bugs caught in RxInfer * fix documentation build * add pretty output for input argument records * 🤖 Auto-format Julia code (#597) Co-authored-by: bvdmitri <6557701+bvdmitri@users.noreply.github.com> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: bvdmitri <6557701+bvdmitri@users.noreply.github.com> * checkpoint * checkpoint * checkpoint * make format * fix some tests by hand * fix more tests by hand * local update * fix more tests * rename vague messagein/messageout * make format * major documentation upgrade * update the CHANGELOG and migration guide * structural documentation update, add concepts * improve documentation for existing pages about specific nodes * update helpers documentation * add concepts pages * finish the documentation overhaul * update the docs * update the version to 6.0.0 * rename pipeline to stream postprocessors * update the scheduler to use stream postprocessors instead * add postprocess marginal stream * simplify API even further * update docs * make format * fix typo in mixture nodes * update * add pre_rule_annotations * add logo, update README.md * make logo half the size in README --------- Co-authored-by: Fons van der Plas <fonsvdplas@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: bvdmitri <6557701+bvdmitri@users.noreply.github.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This PR changes the generation of trace ids (also renamed to span ids) in order to remove massive performance hit for certain models. In my benchmark, the generation of trace IDs affected benchmarks by a factor of 7! And this was even without enabling the callbacks. The current PR changes the implementation a bit by calling an auxiliary function
ReactiveMP.generate_span_idthat would simply returnnothingif callbacks are alsonothing. That fixes the performance hit in case when callbacks are not set, but otherwise it still usesuuid4(definitely the culprit). We may want to use a simple global Int counter for that? Not sure if it would be faster though since this counter would need to be guarded with semaphores to protect from two threads generating the same span ID.@fonsp wdyt?