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 more refinements #24

Merged
merged 9 commits into from Jul 30, 2020
Merged

Conversation

josh-127
Copy link
Contributor

@josh-127 josh-127 commented Jul 24, 2020

Description

This pull request includes various improvements, bug fixes, and refactorings.

Additions

Added command history

Pressing the up arrow recalls the previously ran command, while the down arrow key recalls the next.

Changes

Decoupled AgentLoader from method tracer

AgentLoader is now serving many different tracers, it makes sense to decouple this class from the method tracer. This keep the project organized into a small and tidy tree of package dependencies.

Replaced global fuzzy searcher function with local instances

This was just a small tweak, and it was made to reduce the API surface area of Matcher.kt.

Improved method tracer autocomplete

  • Autocomplete for untrace will now only display classees that are currently or have previously been traced.
  • Added wildcard (*) autocomplete suggestion.

Replace CLI enter key hack with a proper solution

TextFieldWithCompletion instances now use one line mode and can respond to enter key events. This will be useful later on for command history, which requires listening to the up and down arrow keys.

Bug Fixes

Fix partial tracing not working on exact methods

  • Steps to reproduce:
    1. Open up method tracer
    2. Run "trace count tracer"
  • Result: Tracer will also trace wall time, even though only we only intended to trace call count.
  • Cause: traceAndRetransform on exact methods will always call TracerConfig.trace will TracepointFlags.TRACE_ALL as the parameter.
  • Fix: Added a flags parameter to traceAndRetransform

Fix partial tracing corrupting wall time values

  • Steps to reproduce:
    1. Open up method tracer
    2. Run "trace count tracer"
  • Result: Tracer will corrupt the wall time value.
  • Cause: CallTreeBuilder::buildAndReset accumulated wall time values regardless of tracepoint flag values.
  • Fix: Checked tracepoint flags before accumulating wall time values.

Fix parameter value tracing corner case

  • Description: Given this call tree, the expected wall time for f(2) is 15 milliseconds:
f(1), 10 ms
    f(2), 5 ms
f(2), 10 ms
  • Fix: Call trees now refer to an instance of MethodCall, where MethodCall consists of a tracepoint and a list of arguments. ArgSets have been removed.

@josh-127 josh-127 force-pushed the multi/final-touches branch 3 times, most recently from 5b322e1 to d65cc59 Compare July 24, 2020 20:35
@josh-127 josh-127 marked this pull request as ready for review July 27, 2020 16:25
josh-127 added 7 commits July 28, 2020 10:27
AgentLoader is now serving many different tracers, so it makes sense to
decouple this class from the method tracer. This organizes the project
into a small and tidy tree of package dependencies.
* Autocomplete for `untrace` will now only display classes that are
currently or have previously been traced.

* Added wildcard (`*`) autocomplete suggestion.
There were two partial tracing bugs.

----------

Steps to reproduce the first bug:
1. Open up method tracer
2. Run "trace count tracer"

Result: Tracer will also trace wall time, even though only we only
intended to trace call count.

Cause: traceAndRetransform on exact methods will always call
TracerConfig.trace will TracepointFlags.TRACE_ALL as the parameter.

Fix: Added a flags parameter to traceAndRetransform

----------

Steps to reproduce the second bug:
1. Open up method tracer
2. Run "trace count tracer"

Result: Tracer will corrupt the wall time value.

Cause: CallTreeBuilder::buildAndReset accumulated wall time values
regardless of tracepoint flag values.

Fix: Checked tracepoint flags before accumulating wall time values.
Pressing the up arrow recalls the previously ran command, while the down
arrow key recalls the next.
* Fixed key binding for recalling next command

* Fixed all methods getting traced when tracing the same method twice.

* Fixed instrumentation getting initialized more than once
@gharrma gharrma merged commit 749e05f into google:master Jul 30, 2020
@josh-127 josh-127 deleted the multi/final-touches branch July 31, 2020 06:53
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

2 participants