Skip to content

Optimize config.lookup_package check in trace_end #268

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

Merged
merged 1 commit into from
Aug 17, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions lib/appmap/hook.rb
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,6 @@ def trace_end(trace_point)

hook = lambda do |hook_cls|
lambda do |method_id|
# Don't try and trace the AppMap methods or there will be
# a stack overflow in the defined hook method.
next if %w[Marshal AppMap ActiveSupport].member?((hook_cls&.name || '').split('::')[0])

next if method_id == :call

method = \
begin
hook_cls.instance_method(method_id)
Expand All @@ -197,6 +191,16 @@ def trace_end(trace_point)
next
end

package = config.lookup_package(hook_cls, method)
# doing this check first returned early in 98.7% of cases in sample_app_6th_ed
next unless package

# Don't try and trace the AppMap methods or there will be
# a stack overflow in the defined hook method.
next if %w[Marshal AppMap ActiveSupport].member?((hook_cls&.name || '').split('::')[0])

next if method_id == :call

next if self.class.already_hooked?(method)

warn "AppMap: Examining #{hook_cls} #{method.name}" if LOG
Expand All @@ -206,9 +210,6 @@ def trace_end(trace_point)
# TODO: Figure out how to tell the difference?
next unless disasm

package = config.lookup_package(hook_cls, method)
next unless package

package.handler_class.new(package, hook_cls, method).activate
end
end
Expand Down