messages sent to actor are not boxed anymore#370
messages sent to actor are not boxed anymore#370Kannen wants to merge 10 commits intoslawlor:mainfrom
Conversation
|
I have written some benchmarks, it improve OutputPort message dispatching by 50% and concurrent processing and message sending by 22%. |
|
Just to show you how interesting in those changes I propose:
And speed enhancement, this is the comparison of the main branch with the fusion of the changes I propose: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #370 +/- ##
==========================================
+ Coverage 82.48% 82.62% +0.14%
==========================================
Files 71 71
Lines 12919 12984 +65
==========================================
+ Hits 10656 10728 +72
+ Misses 2263 2256 -7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…d check ordering**
d26a55b to
69ef01a
Compare
|
The difference shown in code coverage are due to the fact I have move around some piece of code from one file to the other. This does not change the number of branches tested. |
Those function may only be called in extraordinary conditions: the actor must be killed during the call of send_message.
| } | ||
|
|
||
| #[allow(elided_named_lifetimes)] | ||
| #[allow(mismatched_lifetime_syntaxes)] |
| /// constraints | ||
| pub struct BoxedMessage { | ||
| pub(crate) msg: Option<Box<dyn Any + Send>>, | ||
| pub struct BoxedMessage<T: Any + Send> { |
There was a problem hiding this comment.
Given this is a semver break anyways, we'll need a major release to do this, and should just get rid of BoxedMessage.
It's a poor name given the message would no longer be boxed in this case.
| Message(BoxedMessage), | ||
| Message(BoxedMessage<T>), | ||
| } | ||
| pub(crate) trait GenericInputPort: Sync + Send + Any + 'static { |
There was a problem hiding this comment.
why does this need to be a trait? Is this just for ergonomics? These seem like blanket impl helper functions, I don't see the need for this here
There was a problem hiding this comment.
nevermind looking more closely at the PR, I see the reasoning.
This is an extension of the message boxing approach proposed in PR #370. This however continues the improvements to remove additional type information that isn't necessary. This diff includes the removal of the BoxedMessage struct in favor of a crate-private RactorMessage trait which adds the necessary extension methods, only within the `ractor` context so as the privatize that functionality, as it was an unintentional leak anyways. This of course is a semver break as a public type is going away, but it should have been unused anyways in all contexts. Additionally `BoxedDowncastErr` is renamed to `DowncastErr` as we are no longer boxing the message, but again this error variant shouldn't really be used publicly. Will re-run the performance benchmarks present in #370 and past as a PR comment once ready
This is an extension of the message boxing approach proposed in PR #370. This however continues the improvements to remove additional type information that isn't necessary. This diff includes the removal of the BoxedMessage struct in favor of a crate-private RactorMessage trait which adds the necessary extension methods, only within the `ractor` context so as the privatize that functionality, as it was an unintentional leak anyways. This of course is a semver break as a public type is going away, but it should have been unused anyways in all contexts. Additionally `BoxedDowncastErr` is renamed to `DowncastErr` as we are no longer boxing the message, but again this error variant shouldn't really be used publicly. Will re-run the performance benchmarks present in #370 and past as a PR comment once ready
This is an extension of the message boxing approach proposed in PR #370. This however continues the improvements to remove additional type information that isn't necessary. This diff includes the removal of the BoxedMessage struct in favor of a crate-private RactorMessage trait which adds the necessary extension methods, only within the `ractor` context so as the privatize that functionality, as it was an unintentional leak anyways. This of course is a semver break as a public type is going away, but it should have been unused anyways in all contexts. Additionally `BoxedDowncastErr` is renamed to `DowncastErr` as we are no longer boxing the message, but again this error variant shouldn't really be used publicly. Will re-run the performance benchmarks present in #370 and past as a PR comment once ready
This is an extension of the message boxing approach proposed in PR #370. This however continues the improvements to remove additional type information that isn't necessary. This diff includes the removal of the BoxedMessage struct in favor of a crate-private RactorMessage trait which adds the necessary extension methods, only within the `ractor` context so as the privatize that functionality, as it was an unintentional leak anyways. This of course is a semver break as a public type is going away, but it should have been unused anyways in all contexts. Additionally `BoxedDowncastErr` is renamed to `DowncastErr` as we are no longer boxing the message, but again this error variant shouldn't really be used publicly. Will re-run the performance benchmarks present in #370 and past as a PR comment once ready
This is an extension of the message boxing approach proposed in PR #370. This however continues the improvements to remove additional type information that isn't necessary. This diff includes the removal of the BoxedMessage struct in favor of a crate-private RactorMessage trait which adds the necessary extension methods, only within the `ractor` context so as the privatize that functionality, as it was an unintentional leak anyways. This of course is a semver break as a public type is going away, but it should have been unused anyways in all contexts. Additionally `BoxedDowncastErr` is renamed to `DowncastErr` as we are no longer boxing the message, but again this error variant shouldn't really be used publicly. Will re-run the performance benchmarks present in #370 and past as a PR comment once ready
This is an extension of the message boxing approach proposed in PR #370. This however continues the improvements to remove additional type information that isn't necessary. This diff includes the removal of the BoxedMessage struct in favor of a crate-private RactorMessage trait which adds the necessary extension methods, only within the `ractor` context so as the privatize that functionality, as it was an unintentional leak anyways. This of course is a semver break as a public type is going away, but it should have been unused anyways in all contexts. Additionally `BoxedDowncastErr` is renamed to `DowncastErr` as we are no longer boxing the message, but again this error variant shouldn't really be used publicly. Will re-run the performance benchmarks present in #370 and past as a PR comment once ready
|
@Kannen I can't reproduce your savings (outside maybe of output ports), but I'm dubious if this saves much on processing speed of messages in reality. Have you run the benchmarks multiple times to avoid jitter? You need to be doing it on a quiet host too so there's limited contention. See my results in #387, which shows there's little difference in the regular suite of tests. |
This is an extension of the message boxing approach proposed in PR #370. This however continues the improvements to remove additional type information that isn't necessary. This diff includes the removal of the BoxedMessage struct in favor of a crate-private RactorMessage trait which adds the necessary extension methods, only within the `ractor` context so as the privatize that functionality, as it was an unintentional leak anyways. This of course is a semver break as a public type is going away, but it should have been unused anyways in all contexts. Additionally `BoxedDowncastErr` is renamed to `DowncastErr` as we are no longer boxing the message, but again this error variant shouldn't really be used publicly. Will re-run the performance benchmarks present in #370 and past as a PR comment once ready
This is an extension of the message boxing approach proposed in PR #370. This however continues the improvements to remove additional type information that isn't necessary. This diff includes the removal of the BoxedMessage struct in favor of a crate-private RactorMessage trait which adds the necessary extension methods, only within the `ractor` context so as the privatize that functionality, as it was an unintentional leak anyways. This of course is a semver break as a public type is going away, but it should have been unused anyways in all contexts. Additionally `BoxedDowncastErr` is renamed to `DowncastErr` as we are no longer boxing the message, but again this error variant shouldn't really be used publicly. Will re-run the performance benchmarks present in #370 and past as a PR comment once ready
This is an extension of the message boxing approach proposed in PR #370. This however continues the improvements to remove additional type information that isn't necessary. This diff includes the removal of the BoxedMessage struct in favor of a crate-private RactorMessage trait which adds the necessary extension methods, only within the `ractor` context so as the privatize that functionality, as it was an unintentional leak anyways. This of course is a semver break as a public type is going away, but it should have been unused anyways in all contexts. Additionally `BoxedDowncastErr` is renamed to `DowncastErr` as we are no longer boxing the message, but again this error variant shouldn't really be used publicly. Will re-run the performance benchmarks present in #370 and past as a PR comment once ready
This is an extension of the message boxing approach proposed in PR #370. This however continues the improvements to remove additional type information that isn't necessary. This diff includes the removal of the BoxedMessage struct in favor of a crate-private RactorMessage trait which adds the necessary extension methods, only within the `ractor` context so as the privatize that functionality, as it was an unintentional leak anyways. This of course is a semver break as a public type is going away, but it should have been unused anyways in all contexts. Additionally `BoxedDowncastErr` is renamed to `DowncastErr` as we are no longer boxing the message, but again this error variant shouldn't really be used publicly. Will re-run the performance benchmarks present in #370 and past as a PR comment once ready
This is an extension of the message boxing approach proposed in PR #370. This however continues the improvements to remove additional type information that isn't necessary. This diff includes the removal of the BoxedMessage struct in favor of a crate-private RactorMessage trait which adds the necessary extension methods, only within the `ractor` context so as the privatize that functionality, as it was an unintentional leak anyways. This of course is a semver break as a public type is going away, but it should have been unused anyways in all contexts. Additionally `BoxedDowncastErr` is renamed to `DowncastErr` as we are no longer boxing the message, but again this error variant shouldn't really be used publicly. Will re-run the performance benchmarks present in #370 and past as a PR comment once ready
This is an extension of the message boxing approach proposed in PR #370. This however continues the improvements to remove additional type information that isn't necessary. This diff includes the removal of the BoxedMessage struct in favor of a crate-private RactorMessage trait which adds the necessary extension methods, only within the `ractor` context so as the privatize that functionality, as it was an unintentional leak anyways. This of course is a semver break as a public type is going away, but it should have been unused anyways in all contexts. Additionally `BoxedDowncastErr` is renamed to `DowncastErr` as we are no longer boxing the message, but again this error variant shouldn't really be used publicly. Will re-run the performance benchmarks present in #370 and past as a PR comment once ready
This is an extension of the message boxing approach proposed in PR #370. This however continues the improvements to remove additional type information that isn't necessary. This diff includes the removal of the BoxedMessage struct in favor of a crate-private RactorMessage trait which adds the necessary extension methods, only within the `ractor` context so as the privatize that functionality, as it was an unintentional leak anyways. This of course is a semver break as a public type is going away, but it should have been unused anyways in all contexts. Additionally `BoxedDowncastErr` is renamed to `DowncastErr` as we are no longer boxing the message, but again this error variant shouldn't really be used publicly. Will re-run the performance benchmarks present in #370 and past as a PR comment once ready
This is an extension of the message boxing approach proposed in PR #370. This however continues the improvements to remove additional type information that isn't necessary. This diff includes the removal of the BoxedMessage struct in favor of a crate-private RactorMessage trait which adds the necessary extension methods, only within the `ractor` context so as the privatize that functionality, as it was an unintentional leak anyways. This of course is a semver break as a public type is going away, but it should have been unused anyways in all contexts. Additionally `BoxedDowncastErr` is renamed to `DowncastErr` as we are no longer boxing the message, but again this error variant shouldn't really be used publicly. Will re-run the performance benchmarks present in #370 and past as a PR comment once ready
This is an extension of the message boxing approach proposed in PR #370. This however continues the improvements to remove additional type information that isn't necessary. This diff includes the removal of the BoxedMessage struct in favor of a crate-private RactorMessage trait which adds the necessary extension methods, only within the `ractor` context so as the privatize that functionality, as it was an unintentional leak anyways. This of course is a semver break as a public type is going away, but it should have been unused anyways in all contexts. Additionally `BoxedDowncastErr` is renamed to `DowncastErr` as we are no longer boxing the message, but again this error variant shouldn't really be used publicly. Will re-run the performance benchmarks present in #370 and past as a PR comment once ready
This is an extension of the message boxing approach proposed in PR #370. This however continues the improvements to remove additional type information that isn't necessary. This diff includes the removal of the BoxedMessage struct in favor of a crate-private RactorMessage trait which adds the necessary extension methods, only within the `ractor` context so as the privatize that functionality, as it was an unintentional leak anyways. This of course is a semver break as a public type is going away, but it should have been unused anyways in all contexts. Additionally `BoxedDowncastErr` is renamed to `DowncastErr` as we are no longer boxing the message, but again this error variant shouldn't really be used publicly. Will re-run the performance benchmarks present in #370 and past as a PR comment once ready
This is an extension of the message boxing approach proposed in PR #370. This however continues the improvements to remove additional type information that isn't necessary. This diff includes the removal of the BoxedMessage struct in favor of a crate-private RactorMessage trait which adds the necessary extension methods, only within the `ractor` context so as the privatize that functionality, as it was an unintentional leak anyways. This of course is a semver break as a public type is going away, but it should have been unused anyways in all contexts. Additionally `BoxedDowncastErr` is renamed to `DowncastErr` as we are no longer boxing the message, but again this error variant shouldn't really be used publicly. Will re-run the performance benchmarks present in #370 and past as a PR comment once ready
While navigating inside the code I realized that every message sent to local actor are boxed.
So I made a set of simple changes so that message sent to actor are directly sent to the mpsc channel without being boxed. This avoid one allocation per message.
The benchmarks show improvements. Moreover I think a more realistic benchmark would show much more improvement as avoiding the allocation should enhance cache locality during real code execution.
In this pull request I have used unsafe code, as it avoid to do a redundant type_id check, but it can implemented with 100% with safe code, at the cost of a call to Any::downcast_ref at each message sent.