Implement suppress warnings on for Unused hint and codeAction for suppress warnings annotations

[Achal1607]

Unused suppress warning was not showing up in netbeans IDE, so fixed that.
Netbeans already had option for providing suppress warnings hint in the sub-menu. So, leveraged that piece of code to extend this capability in the VS Code CodeActions as well.
Introduced suppress warnings hint in vscode.

In the above image. "- >" represents subfixes.

For more info refer: oracle/javavscode#96

[lahodaj]

I think @dbalek or @sdedic would be better reviewers than me, as I've been too involved in the discussion on this patch development.

[neilcsmith-net]

@lahodaj sure, but you're also marked as the author on HintsController and HintsControllerImpl so you seemed a good person to suggest an alternative to reflection with regard to @mbien comment (which I happen to agree is something of a red flag)

[Achal1607]

Can @dbalek or @sdedic review this PR and provide some suggestions to avoid use of reflection or any more changes that has to be done?

[mbien]

@Achal1607 question: why was this change made?

I ran some benchmarks which were based on generated files I used for #4142 + #4501

very large files (10K fields and getters) with many positive unused hits performed slightly worse:
PR:

FINE [org.netbeans.modules.java.hints.spiimpl.hints.HintsInvoker]: {time=69114.52ms, invocations=20003, cancelled=false}: org.netbeans.modules.java.hints.bugs.Unused
FINE [org.netbeans.modules.java.hints.spiimpl.hints.HintsInvoker]: hint processing complete

master:

FINE [org.netbeans.modules.java.hints.spiimpl.hints.HintsInvoker]: {time=62578.07ms, invocations=1, cancelled=false}: org.netbeans.modules.java.hints.bugs.Unused
FINE [org.netbeans.modules.java.hints.spiimpl.hints.HintsInvoker]: hint processing complete

the more concerning bit is this though. a 3k fields file which are not unused:

PR:

FINE [org.netbeans.modules.java.hints.spiimpl.hints.HintsInvoker]: {time=565.49ms, invocations=6003, cancelled=false}: org.netbeans.modules.java.hints.bugs.Unused
FINE [org.netbeans.modules.java.hints.spiimpl.hints.HintsInvoker]: hint processing complete

master:

FINE [org.netbeans.modules.java.hints.spiimpl.hints.HintsInvoker]: {time=0.11ms, invocations=1, cancelled=false}: org.netbeans.modules.java.hints.bugs.Unused
FINE [org.netbeans.modules.java.hints.spiimpl.hints.HintsInvoker]: hint processing complete

[Achal1607]

Thanks so much @mbien for running performance tests, this change is made so that it can honour @SuppressWarnings("unused") otherwise currently suppress warning annotation for unused variables is not working. For more info please refer to oracle/javavscode#96

[mbien]

ah got it, the annotation has to anchor on the trigger. I didn't expect that this would matter - makes sense though.

A bit of a shame that this makes the not-unused case so much slower.

[Achal1607]

I am also a bit of surprised by the performance degradation in the not-unused case.

[mbien]

the method is going to be called a few k times on large files, compared to one time before the change. But it seems we are lucky here and a simple fast-path is able to get us closer to the old state:

I added this after findUnused(...) which is already caching results internally

        if (unused.isEmpty()) {
            return null;
        }

null will lead to the fast path in CodeHintProviderImpl#createErrors.

and we get:

FINE [org.netbeans.modules.java.hints.spiimpl.hints.HintsInvoker]: {time=0.83ms, invocations=6003, cancelled=false}: org.netbeans.modules.java.hints.bugs.Unused
FINE [org.netbeans.modules.java.hints.spiimpl.hints.HintsInvoker]: hint processing complete

At some point we should take a look at this method and check what is causing the slowdown there.

[mbien]

it turns out that reading values from Preferences in hot code causes performance problems due to the need of a read lock. This is the reason why the fast path made things so much faster in the not-unused case.

here a commit with more changes: mbien@eb1dc47

feel free to squash with your PR

[mbien]

can we change the PR title to "Implement suppress warnings on for Unused hint" or something similar? I think this makes more sense for the NetBeans release notes which are generated from PR titles, I suppose javavscode has their own release notes.

[Achal1607]

@mbien updated PR title

[Achal1607]

Thank you so much @mbien for helping in fixing the performance issue with this PR, I have incorporated your patch as well in this PR and squashed the PR.

[mbien]

@neilcsmith-net does the do not merge Don't merge this PR, it is not ready or just demonstration purposes. tag still apply?

[mbien]

tested the hint suppression with NetBeans and it worked fine, thanks!

edit: found https://issues.apache.org/jira/browse/NETBEANS-5379 on the old jira which would be closed by this PR

[mbien]

@Achal1607 small small conflict due to #7579 unfortunately. I saw that whitespace change and thought gh would be able to merge, but apparently not.

[mbien]

[PR branch]
git pull --rebase https://github.com/apache/netbeans.git master
[fix merge conflict]
git add java/java.hints/src/org/netbeans/modules/java/hints/infrastructure/JavaErrorProvider.java
git rebase --continue

should work I think. It would put the commit on top of latest master.

[Achal1607]

Thank you so much @mbien for helping out on this PR. I have resolved the merge conflicts. Thanks.