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

Make attrs position-aware #8215

Open
wants to merge 2 commits into
base: 9.5-dev
Choose a base branch
from

Conversation

headius
Copy link
Member

@headius headius commented Apr 25, 2024

This is a proof-of-concept for adding the definition location (from source where attr, attr_reader, attr_accessor or attr_writer was called) into all attr-style methods, to address the missing information reported in #8079.

A benchmark is included, and shows how severely this impacts the creation of attrs, due to the stack walking involved:

[] jruby $ jruby bench/bench_attr_reader.rb 
jruby 10.0.0.0-SNAPSHOT (3.4.0) 2024-04-25 feb5f4c851 OpenJDK 64-Bit Server VM 17.0.5+8 on 17.0.5+8 +jit [arm64-darwin]
Warming up --------------------------------------
         attr_reader    11.853k i/100ms
Calculating -------------------------------------
         attr_reader    128.672k (± 0.5%) i/s -    651.915k in   5.066593s
[] jruby $ (chruby jruby-9.4.6.0 ; jruby bench/bench_attr_reader.rb)
Warming up --------------------------------------
         attr_reader   960.880k i/100ms
Calculating -------------------------------------
         attr_reader      9.701M (± 0.4%) i/s -     49.005M in   5.051463s

Using a specialized call site would make this largely free, similar to how #8170 avoids frame access for block_given? but this example demonstrates one (slow) way to solve the bug.

This patch adds filename and line number information to all attr
methods using our stack walker logic. It works correctly, but it
also imposes a severe performance impact on attr definition, due
to the cost of materializing the JVM stack trace for mining. Even
when using the StackWalker API on a recent JVM, this performs
more than 100x worse per attr defined.

A different solution is probably needed.
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.

None yet

1 participant