From f4d06f195d583794313943e73b1b31d28151b85c Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Thu, 25 Jul 2024 13:30:56 +0800 Subject: [PATCH] PERF: Avoid using `ObjectSpace.each_object` in `Jobs::Onceoff.enqueue_all` (#28072) We are investigating a memory leak in Sidekiq and saw the following line when comparing heap dumps over time. `Allocated IMEMO 14775 objects of size 591000/7389528 (in bytes) at: /var/www/discourse/app/jobs/onceoff/onceoff.rb:36` That line in question was doing a `.select { |klass| klass < self }` on `ObjectSpace.each_object(Class)`. This for some reason is allocating a whole bunch of `IMEMO` objects which are instruction sequence objects. Instead of diving deeper into why this might be leaking, we can just save our time by switching to an implementation that is more efficient and does not require looping through a ton of objects. --- app/jobs/onceoff/onceoff.rb | 20 +++++++++++++------- spec/integrity/onceoff_integrity_spec.rb | 5 +---- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/app/jobs/onceoff/onceoff.rb b/app/jobs/onceoff/onceoff.rb index a8a75421468..40ff504714e 100644 --- a/app/jobs/onceoff/onceoff.rb +++ b/app/jobs/onceoff/onceoff.rb @@ -3,6 +3,15 @@ class Jobs::Onceoff < ::Jobs::Base sidekiq_options retry: false + class << self + attr_reader :onceoff_job_klasses + + def inherited(klass) + @onceoff_job_klasses ||= Set.new + @onceoff_job_klasses << klass + end + end + def self.name_for(klass) klass.name.sub(/\AJobs\:\:/, "") end @@ -31,12 +40,9 @@ class Jobs::Onceoff < ::Jobs::Base def self.enqueue_all previously_ran = OnceoffLog.pluck(:job_name).uniq - ObjectSpace - .each_object(Class) - .select { |klass| klass < self } - .each do |klass| - job_name = name_for(klass) - Jobs.enqueue(job_name.underscore.to_sym) if previously_ran.exclude?(job_name) - end + self.onceoff_job_klasses.each do |klass| + job_name = name_for(klass) + Jobs.enqueue(job_name.underscore.to_sym) if previously_ran.exclude?(job_name) + end end end diff --git a/spec/integrity/onceoff_integrity_spec.rb b/spec/integrity/onceoff_integrity_spec.rb index 2952cfa1be8..1b2fb880875 100644 --- a/spec/integrity/onceoff_integrity_spec.rb +++ b/spec/integrity/onceoff_integrity_spec.rb @@ -7,9 +7,6 @@ RSpec.describe ::Jobs::Onceoff do require_relative "../../app/jobs/onceoff/" + File.basename(f) end - ObjectSpace - .each_object(Class) - .select { |klass| klass.superclass == ::Jobs::Onceoff } - .each { |job| job.new.execute_onceoff(nil) } + described_class.onceoff_job_klasses.each { |job| job.new.execute_onceoff(nil) } end end