Skip to content

Mix PageableResponse into Seahorse::Client::Response instead of extending each response - #3425

Open
ikraamg wants to merge 1 commit into
aws:version-3from
ikraamg:pageable-response-without-extend
Open

ikraamg wants to merge 1 commit into
aws:version-3from
ikraamg:pageable-response-without-extend

Conversation

@ikraamg

@ikraamg ikraamg commented Sep 21, 2026

Copy link
Copy Markdown

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Aws::PageableResponse.apply extends every Seahorse::Client::Response with PageableResponse::Extension. Kernel#extend gives the object a singleton class and clears Ruby's method cache for every method the module defines (each, count, to_h, to_json, as_json, respond_to?, and all of Enumerable via include Enumerable), process-wide, on every request. Rails and its gems ask objects respond_to?(:each) / respond_to?(:to_h) constantly, so under YJIT each request discards every compiled block that assumed one of those names was absent.

This is the method-cache twin of #2670 (which moved the constants out of the module so extend would not bust the constant cache); the comment on Extension still explains that reasoning, and this change applies the same thinking to the method cache.

Measured in a Rails app that signs one S3 URL per request (the presigner builds a response through the same handler stack):

per presigned URL invalidate_method_lookup blocks recompiled JIT code regenerated
version-3 4.0 4.0 2.3 KB
this branch 0 0 0

In production, 10 Puma workers on Ruby 4.0.6 with YJIT reached ~122 MiB of the default 128 MiB --yjit-mem-size after nine hours, with ~3 invalidations a second per worker and 42% of all blocks ever compiled invalidated; at the limit YJIT stops compiling. A gdb breakpoint on rb_yjit_cme_invalidate traced every hit to rb_obj_extend → rb_include_module → invalidate_negative_cache.

The change mixes Extension into Seahorse::Client::Response once, when PageableResponse loads, and makes apply skip extend for an object whose class already includes it. Every response keeps the same interface (each, next_page?, count, …) and apply still works on arbitrary objects, so the existing constant-cache spec is unchanged. The new spec asserts that applying to a response creates no T_CLASS/T_ICLASS, and that a plain Seahorse::Client::Response is already pageable.

bundle exec rspec gems/aws-sdk-core/spec (2427 examples) and gems/aws-sdk-s3/spec (1660 examples) pass on Ruby 4.0.6.

Standalone reproduction (released gem, no network): gem install aws-sdk-s3 && ruby --yjit --yjit-stats repro.rb

Output against aws-sdk-core 3.257.0 on Ruby 4.0.6 (the 4 classes still allocated in the fixed run are the presigner's own Class.new(Handler) blocks, unrelated to this change):

current  per presigned URL: 4 YJIT invalidations, 4.02 blocks recompiled, 2277 bytes of JIT code, 8.0 classes allocated
fixed    per presigned URL: 0 YJIT invalidations, 0.00 blocks recompiled, 0 bytes of JIT code, 4.0 classes allocated
# Shows that Aws::PageableResponse.apply's per-response `extend` invalidates YJIT code and
# allocates a class per response, and that mixing the extension into the response class once fixes it.
#
#   gem install aws-sdk-s3
#   ruby --yjit --yjit-stats pageable_response_extend_repro.rb
#
# Needs only a stubbed client: the presigner builds a response through the same handler stack
# without any network. The "fixed" run applies the proposed change in-process.
require "aws-sdk-s3"

def measure(label)
  client = Aws::S3::Client.new(region: "us-east-1", access_key_id: "a", secret_access_key: "b",
                               stub_responses: true, force_path_style: true)
  presigner = Aws::S3::Presigner.new(client: client)
  sign = -> { presigner.presigned_url(:get_object, bucket: "b", key: "k", expires_in: 300) }
  # Stand-in for a framework: hot code asking plain objects whether they quack like the extension's methods.
  plain = Object.new
  quack = ->(o) { o.respond_to?(:each) || o.respond_to?(:to_h) || o.respond_to?(:as_json) || o.respond_to?(:count) }
  20_000.times { quack.call(plain) }
  200.times { sign.call; 50.times { quack.call(plain) } }

  stats_before = RubyVM::YJIT.runtime_stats.slice(:invalidate_method_lookup, :compiled_block_count, :code_region_size)
  GC.disable
  classes_before = ObjectSpace.count_objects.values_at(:T_CLASS, :T_ICLASS).sum
  urls = 1000
  urls.times { sign.call; 50.times { quack.call(plain) } }
  classes = (ObjectSpace.count_objects.values_at(:T_CLASS, :T_ICLASS).sum - classes_before) / urls.to_f
  GC.enable
  stats = RubyVM::YJIT.runtime_stats.slice(:invalidate_method_lookup, :compiled_block_count, :code_region_size)
  per_url = stats.to_h { |k, v| [k, ((v - stats_before[k]) / urls.to_f).round(2)] }
  puts format("%-8s per presigned URL: %d YJIT invalidations, %.2f blocks recompiled, %.0f bytes of JIT code, %.1f classes allocated",
              label, per_url[:invalidate_method_lookup], per_url[:compiled_block_count], per_url[:code_region_size], classes)
end

abort "run with: ruby --yjit --yjit-stats #{__FILE__}" unless RubyVM::YJIT.enabled? && RubyVM::YJIT.runtime_stats.key?(:invalidate_method_lookup)
puts RUBY_DESCRIPTION
puts "aws-sdk-core #{Aws::CORE_GEM_VERSION}"
measure("current")

# The proposed change: mix the extension into the response class once; apply no longer extends such objects.
Seahorse::Client::Response.include(Aws::PageableResponse::Extension)
Aws::PageableResponse.singleton_class.prepend(Module.new do
  def apply(base)
    return super unless Aws::PageableResponse::Extension === base

    base.instance_variable_set(:@last_page, nil)
    base.instance_variable_set(:@more_results, nil)
    base
  end
end)
measure("fixed")

Generated with AI tools (Claude Code), and reviewed by Ikraam Ghoor.

…ding each response

Kernel#extend gives the response a singleton class and clears the method
cache for every method the extension defines (each, count, to_h,
respond_to?, and all of Enumerable), process-wide, on every request.
Under YJIT that discards every compiled block that relied on one of
those names being absent; a Rails app signing one S3 URL per request
measured ~4 invalidations and 2.3 KB of regenerated code per URL, and
its workers reached --yjit-mem-size within hours and stopped compiling.

Including the extension in the response class once keeps the same
interface on every response with no per-response work. apply still
extends other objects, so its constant cache guarantee is unchanged.
@ikraamg
ikraamg requested a review from a team as a code owner September 21, 2026 07:42
@ikraamg ikraamg closed this Sep 21, 2026
@ikraamg ikraamg reopened this Sep 21, 2026
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.

1 participant