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

Could you keep the 'v1.0.0' interface? #2

Closed
michilu opened this issue Aug 27, 2018 · 5 comments
Closed

Could you keep the 'v1.0.0' interface? #2

michilu opened this issue Aug 27, 2018 · 5 comments

Comments

@michilu
Copy link

michilu commented Aug 27, 2018

I like the new interface. Really, I used the semaphore to control concurrency outside the bus.
And, I hope to keeps both the old interface and the new interface.

v1.0.0

func New() MessageBus {

v1.1.0

func New(maxConcurrentCalls int) MessageBus {

Ref: dd40db9#diff-ecf9c6bcdc8d97b7f04226e8857c1108L82

@carldunham
Copy link

carldunham commented Apr 7, 2019

Heh. I was going to suggest the opposite, that the current interface stutters quite a bit. My concern is more aesthetic tho, as Go Stockholm Syndrome kicks in.

@vardius
Copy link
Owner

vardius commented Apr 7, 2019

First of all @michilu sorry for a very late reply. I meant to reply earlier but after i read your issue at work I forgot to do so after work.

The main goal for v1.1.0 was to reduce the amount of goroutine spawns. By spawning goroutines on subscribe once and keeping them alive. In previous versions many goroutines where spawned each time publish method was called.

Also earlier the concurrency was limited on a global level meaning one queue per all handler of all topics. in v1.1.0 it is limited per topic. Each of topics has it's own queue.

I encourage you to contribute. I am sure together we can workout best performance interface. More use cases gives more insides on actual message-bus usability and performance.

As to the benchmark I suppose we could call b.ResetTimer() after loop which would exclude the startup cost of goroutines.

for i := 0; i < subscribersAmount; i++ {

@michilu
Copy link
Author

michilu commented Apr 9, 2019

@vardius Thanks for your reply.
I will think about your suggestion.

My use case that implements with your message-bus is this.
https://github.com/michilu/boilerplate/blob/173e4736f68c188a6300f4a87f8c3de276dddce6/v/bus/bus.go

I want to use a lightweight pub/sub.
Now I implement by channel via genny, like this.
https://github.com/michilu/boilerplate/blob/master/v/topic/topic.go

If you no problem, close this issue.

@vardius
Copy link
Owner

vardius commented Apr 10, 2019

Would be good to know what is the opinion of other ppl as well.

As to your request we could add new constructor something like this

// NewAsync creates new MessageBus that always spawns goroutines
func NewAsync() MessageBus {
	return New(runtime.NumCPU())
}

which would create new bus with preset number of concurrent calls. maybe we could set it to -1 and then not use buffered channels but call handler directly ?

ideally we create strategy interface (async, sync) and based on that we use buffered channels ?
what do you think ?

@vardius
Copy link
Owner

vardius commented Jul 15, 2019

I’m closing this issue because it has been inactive for a while. Please use the https://github.com/vardius/message-bus/releases/tag/v1.0.0 for v1.0.0 interface. Keep in mind that by using older release you loose recent bug fixes and futures.

Thank you!

@vardius vardius closed this as completed Jul 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants