Skip to content

Log points to easily dump content of local variables - #9732

Open
jtulach wants to merge 9 commits into
apache:masterfrom
jtulach:jtulach/LogPoints
Open

jtulach wants to merge 9 commits into
apache:masterfrom
jtulach:jtulach/LogPoints

Conversation

@jtulach

@jtulach jtulach commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor
  • NetBeans IDE already supports log points
    • yet configuring them isn't easy (especially for debugging newbies)
  • this PR proposes an extra action "Add Logpoint"
log point action
  • the log point is just properly configured breakpoint
    • doesn't stop any thread when hit
    • asks to dump all local variables into "debugger console" everytime it is hit
  • e.g. prints multiple times when put into a for loop
log points
  • to make the configuratin easy a new {locals} directive in setPrintText message is recognized
    • to simplify the most common case
    • e.g. when one dumps all local variables of a method
  • new Variable.toFixed() API added to support "snapshotting the variable"

@jtulach jtulach self-assigned this Oct 19, 2026
@jtulach jtulach added Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) debugger labels Oct 19, 2026
@jtulach jtulach added this to the NB33 milestone Oct 19, 2026
io.printWithAction(" = ", null, false);
if (refValue instanceof ObjectVariable obj && !isNull(obj)) {
var sessionRef = new WeakReference<>(session);
io.printWithAction(obj.getValue(), () -> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hyperlink captures an ObjectLocalVariable wrapper that NetBeans reuses and updates. When a loop assigns another object to the same local, earlier links retain their original text but target the newer object. A temporary native test reproduced this: the first hyperlink’s object ID changed from 89 to 97. Capture a stable object reference when creating the link.

@jtulach jtulach Oct 19, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

..temporary native test reporduced this ...

I don't understand. What is the test doing? True tests were missing, but I've just added one in 15eb8c4. It passes. What's the problem with the ObjectLocalVariable? Following code seems to be logging value of fac array properly:

fac array

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for editing your comment, newly using GH PR extension in VS Code and it has some editing specialities....
LogpointReviewTests.zip this is quick and dirty review regression test.

cp LogpointReviewTest.java to java/debugger.jpda.ui/test/unit/src/org/netbeans/modules/debugger/jpda/ui/

cp LogpointReviewMain.java to java/debugger.jpda.ui/test/unit/src/test/
Set: review_jvm_flags="$(cat nbbuild/jms-config/{base,desktop,compiler}.flags | tr '\n' ' ')"
Run then:

ANT_OPTS=-Djava.awt.headless=true \
ant -f java/debugger.jpda.ui/build.xml test-unit \
  -Dtest.includes='**/LogpointReviewTest.class' \
  -Dtest.qa-functional.src.dir=/tmp/pr9732-no-tests \
  "-Dtest.run.args=-ea -Djava.awt.headless=true -XX:+IgnoreUnrecognizedVMOptions $review_jvm_flags"

Should fail with something like: Earlier hyperlink should still target the first object expected:<89> but was:<97>
ID/numbers can differ for your run. Hope this helps

@jtulach jtulach Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the test, @MartinBalin(e)! It is valid and demontrates the problem quite clearly. It has opened my eyes. Added in e644433 - the test really fails the CI.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The e644433 test is fixed by Variable.toFixed() introduced in 4db2f0a

@MartinBalin MartinBalin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left 2 comments

)
@ActionReferences({
@ActionReference(path = "Editors/text/x-java/Popup", position = 1900),
@ActionReference(path = "Editors/AnnotationTypes/BreakpointActionsWithToggleNew", position = 400)

@jtulach jtulach Oct 19, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this Editors/AnnotationTypes/BreakpointActionsWithToggleNew registration we now see the action in the editor gutter of the Java files:

Image

Non-Java editors like PHP don't show this action. It wouldn't work as it is using JPDABreakpoint functionality.

@jtulach
jtulach requested a review from MartinBalin October 19, 2026 18:50
@jtulach
jtulach requested a review from MartinBalin October 26, 2026 06:30
* @return an <em>immutable snapshot</em> of the variable
* @since 3.47
*/
default Variable toFixed() {

@jtulach jtulach Oct 26, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toFixed functionality was there, but it wasn't easily accessible via the API. Making it real, official API.

public void addFixedWatch (String name, Variable variable) {
public void addFixedWatch (String name, Variable orig) {
// Clone the variable to assure that it's unique and sticks to the JDI value.
if (variable instanceof Cloneable) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • previous obscure alternative of Variable.toFixed()
  • the AbstractVariable implemented Cloneable, but as Object.clone() method is protected...
  • reflection had to be used to call it
  • now it becomes simple toFixed() call

@eppleton eppleton left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice feature, and toFixed() fixes the hyperlink identity problem cleanly. I checked a few things that turned out fine: SUSPEND_NONE breakpoints still pause the event thread while the listeners run (BreakpointImpl line 258), so reading the frame in printLocals is safe. The FixedWatchesManager change keeps the old behaviour, because every cloneable Variable (including WatchesModel.JPDAWatchEvaluating) extends AbstractVariable. The gutter popup moves the caret before it opens, so getCurrentLineNumber() returns the right line.

A few comments inline. Small nits:

  • debugger.jpda now overrides Variable.toFixed() from API 3.47, but its dependency on api.debugger.jpda is still 3.18. It does no harm at runtime, but bumping it would keep the dependency honest.
  • The apichanges date is 28 Oct.
  • FixedWatchesManager.addFixedWatch has a double space in = orig.toFixed().
  • The only red CI job is a MoveMethodTest timeout in refactoring.java, which looks unrelated.

Object createMirrorObject();


/** Creates an<em>immutable snapshot</em> of the variable.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The javadoc promises an immutable snapshot of the current state, but for objects the copy only keeps pointing at the same object. Its state is still live.

I checked this locally on the PR head. A local held an object whose int field the program changed from 100 to 101 between two hits of the logpoint. At the second hit, the variable held by the first hit's hyperlink reported 101 (same unique ID in both hits). That fits the code: AbstractObjectVariable.clone() reuses the same JDI ObjectReference, and DebuggetStateListener marks the fields for re-reading as soon as the debuggee resumes. Garbage collection isn't disabled either, so the object can also disappear later.

So for a logpoint, clicking the link from iteration 1 shows the object as it is now, not as it was when the line was printed. Since this becomes public API, I'd describe what it actually guarantees: the copy keeps referring to the current value/object, regardless of what the original variable is reassigned to later. Two minor things: "animmutable" is missing a space, and toFixed might remind people of JavaScript's Number.toFixed.

io.print(" ", null, false);
io.print(refValue.getName(), null, false);
io.print(" = ", null, false);
if (refValue instanceof ObjectVariable ov && !isNull(ov) && ov.toFixed() instanceof ObjectVariable obj) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something to keep in mind about memory (I haven't measured it): every hit creates a copy of each object local, and the console hyperlink keeps it alive. Each copy also registers a (weak) state listener on the debugger in the AbstractObjectVariable constructor. {=expr} already works this way, but {locals} multiplies it by the number of locals, and logpoints are meant to sit in loops. A logpoint in a hot loop would keep adding up for the whole session. Maybe this is acceptable, but it might be worth limiting how many linked hits are kept.

)
@ActionReferences({
@ActionReference(path = "Editors/text/x-java/Popup", position = 1900),
@ActionReference(path = "Editors/AnnotationTypes/BreakpointActionsWithToggleNew", position = 400)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BreakpointActionsWithToggleNew is the popup of the existing Breakpoint annotation (Breakpoint.xml in spi.debugger.ui), so the entry only appears on a line that already has a breakpoint. "Add Logpoint" there adds a second breakpoint on the same line, so the line both stops and logs. Is that intended? On an existing breakpoint, I think users would expect "turn this into a logpoint" (set suspend to none and fill in the print text). The same applies to the editor popup on a line that already has a breakpoint.

}
}

int printLocals = printText.indexOf(localsPattern);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor points about {locals}:

  • It is removed from wherever it appears, and the locals are always printed after the message.
  • Only the first occurrence is removed; a second {locals} is printed literally.

Both are fine if documented, but I couldn't find {locals} mentioned anywhere a user would see it, e.g. near the print text field in the breakpoint customizer.

import org.netbeans.junit.NbTestCase;
import org.openide.util.Utilities;

public class LogpointReviewTest extends NbTestCase {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to have a regression test for the identity problem. Some suggestions for keeping it maintainable:

  • The names LogpointReviewTest/LogpointReviewMain describe how the test came about rather than what it checks.
  • It reads private console internals by reflection (output, ioManager, buffer, text, action) plus the lambda's captured fields, so any refactoring of the console IO will break it.
  • It sets the global test.dir.src property.

Could it go into BreakpointOutputTest, driving printLocals through the new BreakpointOutputWriter? That would at least avoid depending on the console internals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

debugger Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants