chore(api): update benchmarks#2107
Conversation
xuan-cao-swi
requested review from
ahayworth,
arielvalentin,
dazuma,
ericmustin,
fbogsany,
kaylareopelle,
mwear,
plantfansam,
robbkidd and
robertlaurin
as code owners
| spec.add_dependency 'logger' | ||
|
|
||
| spec.add_development_dependency 'benchmark-ipsa', '~> 0.2.0' | ||
| spec.add_development_dependency 'benchmark-ips', '~> 2.14.0' |
There was a problem hiding this comment.
Because we are switching tools why don't we move this to the gemfile?
There was a problem hiding this comment.
I am ok with either put here or in gemfile
There was a problem hiding this comment.
I would rather gem so that we can move towards eliminating the disabling of the corresponding rubocop cop & the updating process is natively handled by renovate.
| end | ||
|
|
||
| Benchmark.ipsa do |x| | ||
| Benchmark.ips do |x| |
There was a problem hiding this comment.
Are you sure that this isn't changing the measurements/data being captured/recorded?
There was a problem hiding this comment.
ipsa only have additional allocations stats from MemoryProfiler.report()
e.g.
Allocations -------------------------------------
name= 0/0 alloc/ret 0/0 strings/ret
set_attribute 0/0 alloc/ret 0/0 strings/ret
add_event 2/2 alloc/ret 0/0 strings/ret
I'm not sure this information is still valuable or relevant, as it doesn't provide meaningful comparisons like ips does (e.g., xxx is 1.45x faster than xxx). ipsa is also quite old and conflicts with using the latest version of ips. If allocation data is truly needed, we could simply inline/copy the relevant MemoryProfiler.report logic rather than depending on an unmaintained gem.
There was a problem hiding this comment.
I am All for switching from an unmaintained gem, my question steemed from reading the pr description.
2 participants
Description
benchmarkswas broken: added the opentelemetry-sdk as test dependency.I don't see the need to keep using
benchmark-ipsafor the additionalmemory_profiler.report(correct me if I am wrong)