From e1a8e5a509fc11c1373367f7399da6c12b51db39 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 10 Dec 2017 21:48:13 -0800 Subject: [PATCH 1/2] Remove allocation tracking code from InfluxDB sampler for performance On GitLab.com, InfluxSampler#sample_objects appears to take 1.2 s or so to iterate through 1059 objects. This had led to delays of a couple hundred milliseconds in processing in the main thread. Remove this code since it's not really being used. Closes gitlab-com/infrastructure#3250 --- ...sh-remove-allocation-tracking-influxdb.yml | 5 ++++ lib/gitlab/metrics/samplers/influx_sampler.rb | 24 ------------------- .../metrics/samplers/influx_sampler_spec.rb | 23 ------------------ 3 files changed, 5 insertions(+), 47 deletions(-) create mode 100644 changelogs/unreleased/sh-remove-allocation-tracking-influxdb.yml diff --git a/changelogs/unreleased/sh-remove-allocation-tracking-influxdb.yml b/changelogs/unreleased/sh-remove-allocation-tracking-influxdb.yml new file mode 100644 index 00000000000..b98573df303 --- /dev/null +++ b/changelogs/unreleased/sh-remove-allocation-tracking-influxdb.yml @@ -0,0 +1,5 @@ +--- +title: Remove allocation tracking code from InfluxDB sampler for performance +merge_request: +author: +type: performance diff --git a/lib/gitlab/metrics/samplers/influx_sampler.rb b/lib/gitlab/metrics/samplers/influx_sampler.rb index f4f9b5ca792..5a0f7f28fc8 100644 --- a/lib/gitlab/metrics/samplers/influx_sampler.rb +++ b/lib/gitlab/metrics/samplers/influx_sampler.rb @@ -27,7 +27,6 @@ module Gitlab def sample sample_memory_usage sample_file_descriptors - sample_objects sample_gc flush @@ -48,29 +47,6 @@ module Gitlab add_metric('file_descriptors', value: System.file_descriptor_count) end - if Metrics.mri? - def sample_objects - sample = Allocations.to_hash - counts = sample.each_with_object({}) do |(klass, count), hash| - name = klass.name - - next unless name - - hash[name] = count - end - - # Symbols aren't allocated so we'll need to add those manually. - counts['Symbol'] = Symbol.all_symbols.length - - counts.each do |name, count| - add_metric('object_counts', { count: count }, type: name) - end - end - else - def sample_objects - end - end - def sample_gc time = GC::Profiler.total_time * 1000.0 stats = GC.stat.merge(total_time: time) diff --git a/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb index 667e4747897..f66451c5188 100644 --- a/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb @@ -21,7 +21,6 @@ describe Gitlab::Metrics::Samplers::InfluxSampler do it 'samples various statistics' do expect(sampler).to receive(:sample_memory_usage) expect(sampler).to receive(:sample_file_descriptors) - expect(sampler).to receive(:sample_objects) expect(sampler).to receive(:sample_gc) expect(sampler).to receive(:flush) @@ -72,28 +71,6 @@ describe Gitlab::Metrics::Samplers::InfluxSampler do end end - if Gitlab::Metrics.mri? - describe '#sample_objects' do - it 'adds a metric containing the amount of allocated objects' do - expect(sampler).to receive(:add_metric) - .with(/object_counts/, an_instance_of(Hash), an_instance_of(Hash)) - .at_least(:once) - .and_call_original - - sampler.sample_objects - end - - it 'ignores classes without a name' do - expect(Allocations).to receive(:to_hash).and_return({ Class.new => 4 }) - - expect(sampler).not_to receive(:add_metric) - .with('object_counts', an_instance_of(Hash), type: nil) - - sampler.sample_objects - end - end - end - describe '#sample_gc' do it 'adds a metric containing garbage collection statistics' do expect(GC::Profiler).to receive(:total_time).and_return(0.24) From 10f5446c336653004b7851af7c67a3ec010ade05 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 11 Dec 2017 06:53:56 -0800 Subject: [PATCH 2/2] Remove RubySampler#sample_objects for performance as well --- lib/gitlab/metrics/samplers/ruby_sampler.rb | 27 ------------------- .../metrics/samplers/ruby_sampler_spec.rb | 23 ---------------- 2 files changed, 50 deletions(-) diff --git a/lib/gitlab/metrics/samplers/ruby_sampler.rb b/lib/gitlab/metrics/samplers/ruby_sampler.rb index f4901be9581..b68800417a2 100644 --- a/lib/gitlab/metrics/samplers/ruby_sampler.rb +++ b/lib/gitlab/metrics/samplers/ruby_sampler.rb @@ -48,7 +48,6 @@ module Gitlab def sample start_time = System.monotonic_time sample_gc - sample_objects metrics[:memory_usage].set(labels, System.memory_usage) metrics[:file_descriptors].set(labels, System.file_descriptor_count) @@ -68,32 +67,6 @@ module Gitlab end end - def sample_objects - list_objects.each do |name, count| - metrics[:objects_total].set(labels.merge(class: name), count) - end - end - - if Metrics.mri? - def list_objects - sample = Allocations.to_hash - counts = sample.each_with_object({}) do |(klass, count), hash| - name = klass.name - - next unless name - - hash[name] = count - end - - # Symbols aren't allocated so we'll need to add those manually. - counts['Symbol'] = Symbol.all_symbols.length - counts - end - else - def list_objects - end - end - def worker_label return {} unless defined?(Unicorn::Worker) diff --git a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb index 53699327da1..375cbf8a9ca 100644 --- a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb @@ -11,7 +11,6 @@ describe Gitlab::Metrics::Samplers::RubySampler do it 'samples various statistics' do expect(Gitlab::Metrics::System).to receive(:memory_usage) expect(Gitlab::Metrics::System).to receive(:file_descriptor_count) - expect(sampler).to receive(:sample_objects) expect(sampler).to receive(:sample_gc) sampler.sample @@ -65,26 +64,4 @@ describe Gitlab::Metrics::Samplers::RubySampler do sampler.sample end end - - if Gitlab::Metrics.mri? - describe '#sample_objects' do - it 'adds a metric containing the amount of allocated objects' do - expect(sampler.metrics[:objects_total]).to receive(:set) - .with(include(class: anything), be > 0) - .at_least(:once) - .and_call_original - - sampler.sample - end - - it 'ignores classes without a name' do - expect(Allocations).to receive(:to_hash).and_return({ Class.new => 4 }) - - expect(sampler.metrics[:objects_total]).not_to receive(:set) - .with(include(class: 'object_counts'), anything) - - sampler.sample - end - end - end end