completion: listing constructors is slow

[Eskibear]

CPU: Intel(R) Core(TM) i7-10700 CPU @ 2.90GHz *8 cores
OS: Windows 10
Test project: a simple maven project with no extra dependency
Use case: complete for new constructor

    public String sayHello() {
        new S‸
    }

profiling details:

Sample response:

Because my CPU is relative powerful, 263ms is not a small number. It can be up to 1s on my other machines.
one item from completion response:

Received response 'textDocument/completion - (108)' in 263ms.
Result: {
    "isIncomplete": false,
    "items": [
...
{
            "label": "SAXException()",
            "kind": 4,
            "detail": "org.xml.sax.SAXException.SAXException()",
            "sortText": "999999163",
            "insertText": "SAXException",
            "insertTextFormat": 2,
            "textEdit": {
                "range": {
                    "start": {
                        "line": 7,
                        "character": 12
                    },
                    "end": {
                        "line": 7,
                        "character": 13
                    }
                },
                "newText": "SAXException()"
            },
            "data": {
                "decl_signature": "Lorg.xml.sax.SAXException;",
                "signature": "()V",
                "name": "SAXException",
                "pid": "65",
                "rid": "7",
                "uri": "file:///C:/Users/yanzh/Desktop/maven-dependency-conflict-demo-master/project-commmon/src/main/java/projectcommon/HelloWorld.java"
            }
...

response of resolve request:

Received response 'completionItem/resolve - (109)' in 6ms.
Result: {
    "label": "SAXException()",
    "kind": 4,
    "detail": "org.xml.sax.SAXException.SAXException()",
    "documentation": {
        "kind": "markdown",
        "value": "Create a new SAXException."
    },
    "sortText": "999999163",
    "insertTextFormat": 2,
    "textEdit": {
        "range": {
            "start": {
                "line": 7,
                "character": 12
            },
            "end": {
                "line": 7,
                "character": 13
            }
        },
        "newText": "SAXException()"
    },
    "additionalTextEdits": [
        {
            "range": {
                "start": {
                    "line": 0,
                    "character": 22
                },
                "end": {
                    "line": 2,
                    "character": 0
                }
            },
            "newText": "\n\nimport org.xml.sax.SAXException;\n\n"
        }
    ]

[Eskibear]

From the responses we can see, we already defer calculation of import statements in resolve stage, which once was the most time-consuming task.

Now the problem is, why below methods cost so much time? Are they necessary? Do we have to calculate it in complete stage?
org.eclipse.jdt.ls.core.internal.contentassist.CompletionProposalReplacementProvider.appendAnonymousClass
org.eclipse.jdt.ls.core.internal.contentassist.CompletionProposalDescriptionProvider.appendUnboundedParameterList
Let's see whether we can improve it.

[Eskibear]

https://github.com/eclipse/eclipse.jdt.ls/blob/64b15c5a9e5b11f62ceb5163ceb6930d5dea7129/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/contentassist/AnonymousTypeCompletionProposal.java#L104

Above line is the culprit, it formats the hard-coded body part for every completion item.

E.g. When we complete for new Runnable, the expected output should be:

Runnable() {

}

It's combined by two parts.

1. the name Runnable
2. the new body () {\n}

The way to calculate new body is to hard-code a string new A() {\n}, format it with current preferences, and remove new A.

I assume the original purpose was to have correct indentation and line delimiters. Formatting is expensive, and the same stuff is repeatedly calulated for all items.

Attaching Call_Tree_CompleteForConstructors.zip

(I remove this line and played for a while, and I didn't see any difference. But of course there must be edge cases untested)

Potential solutions

• Option 1. If the only purpose is to have correct indentation and line end, we can manually format () {\n} instead of calling org.eclipse.jdt.internal.corext.util.CodeFormatterUtil.format, which involves expensivev operations like AST construction, etc.

• Option 2. For each completion, we only do it once for all items. The change is safe because current compilation unit and target string to format are same among all items.

[fbricon]

I don't understand how option 2 works

[Eskibear]

The same new body (){\n\t${0}\n} is formatted for all anonymous constructor candidates. E.g. When cursor is at Test.I, for all available anonymous constructors, the new body is the same, and the formatter options are also the same.

[fbricon]

ok if you find an elegant way to call that part once for all items then I'm all for it

[fbricon]

There's a whole bunch of StringBuffers in that class, moving to StringBuilders won't hurt

[Eskibear]

ok if you find an elegant way to call that part once for all items then I'm all for it

I just recall why we have that part. In the past the new body also contained unimplemented methods (removed for performance concern), and that's why we needed to format.

[testforstephen]

Close it with PR #1854