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

feat(event pool): simplify realization of event pool #616

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ythosa
Copy link
Contributor

@ythosa ythosa commented Apr 21, 2024

Description

I have managed to improve the performance of the pool and improve its readability

Benchmarks

Old

BenchmarkEventPoolOneGoroutine-8        58460538                19.96 ns/op            0 B/op          0 allocs/op
BenchmarkEventPoolManyGoroutines-8           100          10746244 ns/op            4986 B/op         56 allocs/op
BenchmarkEventPoolSlowestPath-8              152           8189235 ns/op           27087 B/op       1034 allocs/op

New

BenchmarkEventPoolOneGoroutine-8        39543708                32.61 ns/op            0 B/op          0 allocs/op
BenchmarkEventPoolManyGoroutines-8           697           1724738 ns/op             876 B/op         15 allocs/op
BenchmarkEventPoolSlowestPath-8             1026           1038654 ns/op           24619 B/op       1007 allocs/op

@ythosa
Copy link
Contributor Author

ythosa commented Apr 21, 2024

@kirillov6 @goshansmails please review!

@ythosa
Copy link
Contributor Author

ythosa commented Apr 22, 2024

btw, i've tested realisation based on sync.Pool, it was too much faster than my new realisation, but the possibility of event dump was lost

@vadimalekseev
Copy link
Collaborator

@ythosa, hello!

You can use BenchmarkLightJsonReadPar to check how the performance has changed. On mac m1 this bench showed following result:
old:

BenchmarkLightJsonReadPar-8            1        2256751708 ns/op         643.06 MB/s     4825008 B/op      48384 allocs/op

new:

BenchmarkLightJsonReadPar-8            1        4433031459 ns/op         327.37 MB/s     4272744 B/op      42659 allocs/op

Also, we can't use sync.Pool because insaneJSON uses own global pool. So, we need to refactor insaneJSON first, then we can test sync.Pool

@ythosa
Copy link
Contributor Author

ythosa commented Apr 26, 2024

Wow, I haven't seen this benchmark. You are very cool...

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

Successfully merging this pull request may close these issues.

2 participants