-
Notifications
You must be signed in to change notification settings - Fork 34
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
Add a Java agent for Trace Debugger #464
Conversation
f8f570d
to
5718572
Compare
3d1f0b1
to
6ba67b5
Compare
@dmitrii-artuhov please rebase the PR onto |
@dmitrii-artuhov, please also add a test to check that the agent works as expected. I suggest adding an additional system property to run the agent without the plugin but printing the execution trace to a specified file, thus providing a testing framework similar to representation tests in Lincheck. Then, you can create a simple project and check that running Java's |
5718572
to
afc5e6e
Compare
.../main/org/jetbrains/kotlinx/lincheck/strategy/managed/modelchecking/ModelCheckingStrategy.kt
Outdated
Show resolved
Hide resolved
/** | ||
* This method is called from the trace-debugger evaluation. | ||
*/ | ||
private fun visualizeTrace(): Array<Any>? = runCatching { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eupp should we have a similar fix for the general-purpose model checker? What is the current status of its integration with the plugin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we would need a similar fix.
Let's maybe keep visualizeTrace
name for a while, because I guess it is also used on the trace-debugger plugin side.
Then once we refactor visualize
for general-purpose mc, we would be able to re-use it here:
private fun visualizeTrace(): Array<Any>? = visualize()
or remove it completely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we provide thread names through visualizeTrace
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also tagging @avpotapov00
...m/test/org/jetbrains/kotlinx/lincheck_test/representation/CustomThreadsRepresentationTest.kt
Outdated
Show resolved
Hide resolved
src/jvm/main/org/jetbrains/kotlinx/lincheck/transformation/TraceDebuggerClassVisitor.kt
Outdated
Show resolved
Hide resolved
src/jvm/main/org/jetbrains/kotlinx/lincheck/transformation/TraceDebuggerClassVisitor.kt
Outdated
Show resolved
Hide resolved
|
||
internal class TraceDebuggerStaticMethodWrapper { | ||
fun callStaticMethod(clazz: Class<*>, method: Method) { | ||
ensureClassHierarchyIsTransformed(clazz.name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the trace debugger mode, let's transform ALL classes, never calling ensureClassHierarchyIsTransformed(..)
and the reflection calls that preceed it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too much classes are transformed I guess. I wasn't able to wait until the trace would be collected and shown by the plugin, it was endlessly spinning with IDEA background task "Collecting trace..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, wow. OK, let's postpone this change then.
@@ -139,6 +139,19 @@ private fun visualize(strategyObject: Any) = runCatching { | |||
visualizeInstance(testObject, objectToNumberMap, continuationToLincheckThreadIdMap, threadToLincheckThreadIdMap) | |||
} | |||
|
|||
/** | |||
* This method is called from the trace-debugger evaluation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please indicate when the plugin calls this method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea when it happens)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please investigate and leave a comment
@@ -229,30 +229,30 @@ internal class ModelCheckingStrategy( | |||
} | |||
|
|||
if (representation.isNotEmpty()) { | |||
representations.add("$type;${node.iThread};${node.callDepth};${node.shouldBeExpanded(false)};${eventId};${representation}") | |||
representations.add("$type;${node.iThread};${node.callDepth};${node.shouldBeExpanded(false)};${eventId};${representation};null") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need this null
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a stack trace element according to plugin, I don't know if it is used aywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm afraid, this change might break the Lincheck plugin and is unnecessary for the Trace Debugger one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@avpotapov00 do you know how this argument is used in the trace debugger plugin?
.../main/org/jetbrains/kotlinx/lincheck/strategy/managed/modelchecking/ModelCheckingStrategy.kt
Outdated
Show resolved
Hide resolved
src/jvm/main/org/jetbrains/kotlinx/lincheck/transformation/TraceDebuggerAgent.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left minor comments, please address them, and the PR is ready to go 🚀
@dmitrii-artuhov Please also check that the Lincheck plugin works with this change. |
6b87f9b
to
e4b8532
Compare
…e it was always 'null'
This PR adds a static agent for trace debugger plugin (though, lincheck's agent is left dynamic, yet). The pair-PR for the plugin could be found here.
This PR is based on #460