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

wrong status in 'language/progressReport' notification when processing call hierarchy requests #1722

Closed
Eskibear opened this issue Apr 13, 2021 · 1 comment · Fixed by #1802 or #2827
Assignees
Milestone

Comments

@Eskibear
Copy link
Contributor

By a glance of the log, I find workDone is 10x larger than totalWork, making status 1000% which is unreasonable. Is it from upstream or set by jdtls itself?

[Trace - 10:53:33 AM] Sending request 'callHierarchy/incomingCalls - (11)'.
Params: {
    "item": {
        "name": "main(String[]) : void",
        "kind": 6,
        "uri": "file:///c%3A/Users/zy199/Desktop/java/Simple/Hello.java",
        "range": {
            "start": {
                "line": 16,
                "character": 4
            },
            "end": {
                "line": 23,
                "character": 5
            }
        },
        "selectionRange": {
            "start": {
                "line": 21,
                "character": 23
            },
            "end": {
                "line": 21,
                "character": 27
            }
        },
        "detail": "Hello"
    }
}


[Trace - 10:53:33 AM] Received notification 'language/progressReport'.
Params: {
    "task": "Background task",
    "status": "0% ",
    "totalWork": 1000,
    "workDone": 0,
    "complete": false
}


[Trace - 10:53:33 AM] Received notification 'language/progressReport'.
Params: {
    "task": "Reconciling...",
    "status": "100% ",
    "totalWork": 1000,
    "workDone": 1000,
    "complete": true
}


[Trace - 10:53:33 AM] Received notification 'language/progressReport'.
Params: {
    "task": "Reconciling...",
    "status": "100% ",
    "totalWork": 1000,
    "workDone": 1000,
    "complete": true
}
[Trace - 10:53:33 AM] Received notification 'language/progressReport'.
Params: {
    "task": "Finding callers...",
    "status": "1000% ",                       //<--------------HERE----------------------
    "totalWork": 100,
    "workDone": 1000,
    "complete": true
}


[Trace - 10:53:34 AM] Received notification 'language/progressReport'.
Params: {
    "task": "Finding callers...",
    "status": "1000% ",                      //<---------------HERE----------------------
    "totalWork": 100,
    "workDone": 1000,
    "complete": true
}


[Trace - 10:53:34 AM] Received response 'callHierarchy/incomingCalls - (11)' in 704ms.
...
@rgrunber
Copy link
Contributor

rgrunber commented Apr 13, 2021

Doing a quick investigation, looks like it's in the CallHierarchyHandler there are some calls to MethodWrapper#getCalls(monitor). We seem to be re-using MethodWrapper calls by caching them as incoming/outgoing. As part of the getCalls(..) logic I see calls to monitor.beginTask(..) which would probably reset the total work (which is what we see). Note that beginTask(..) states Notifies that the main task is beginning. This must only be called once on a given progress monitor instance.

May need to create a monitor subtask before passing it in.

@rgrunber rgrunber self-assigned this Jun 17, 2021
rgrunber added a commit to rgrunber/eclipse.jdt.ls that referenced this issue Jun 17, 2021
- A method should call Submonitor.convert(..) upon receiving its monitor
  in order to correctly allocate its share of the work to be done
- Fixes eclipse-jdtls#1722

Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
rgrunber added a commit that referenced this issue Jun 18, 2021
- A method should call Submonitor.convert(..) upon receiving its monitor
  in order to correctly allocate its share of the work to be done
- Fixes #1722

Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
@rgrunber rgrunber added this to the End June 2021 milestone Jun 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment