Conversation
| Metadata: "test", | ||
| } | ||
| b.Run("SendMessage/"+name, func(b *testing.B) { | ||
| for i := 0; i < 10; i++ { |
There was a problem hiding this comment.
| for i := 0; i < 10; i++ { | |
| for i := 0; i < b.N; i++ { |
|
|
||
| // NewSyncProducerFromClient creates a new SyncProducer using the given client. It is still | ||
| // necessary to call Close() on the underlying client when shutting down this producer. | ||
| func NewSyncProducerFromOptimized(client Client) (SyncProducer, error) { |
There was a problem hiding this comment.
I know this wouldn’t stay the name, but we should either drop all of FromClient or neither word.
| func NewSyncProducerFromOptimized(client Client) (SyncProducer, error) { | |
| func NewSyncProducerFromClientOptimized(client Client) (SyncProducer, error) { |
(That is, I expect this is a demonstration that the sync pool way is better without any downside, and so the original implementation should be converted to this implementation.)
There was a problem hiding this comment.
Yes, i wanted to ran benchmarks and compare them, also i will try to run fork on our production and come back with profiler results comparison.
For now can i just add sync.Pool into SyncProducer, remove SyncProducerOptimized and remove benchmark? Is there any suggestions to additional tests?
There was a problem hiding this comment.
I might suggest starting a different branch for the intended-to-be-merged changes. (That is, incorporating the sync.Pool into SyncProducer, and dropping SyncProducerOptimized.) But keeping this sort of change open so anyone can run their own benchmarks locally has a lot of value, even if the PR would just be closed after merging the intended fix.
There was a problem hiding this comment.
There was a problem hiding this comment.
Linked both of them here
#3107 (comment)
|
Thanks for #3109 - closing this one now |
3 participants
Ran benchmarks on solution with sync.Pool for expectation channels on SyncProducer