[jira] [Comment Edited] (KNOX-792) Fix FindBugs "performance" issues

["Larry McCay (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/KNOX-792?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15707326#comment-15707326 ] 

Larry McCay edited comment on KNOX-792 at 11/30/16 2:53 AM:
------------------------------------------------------------

Hi [~coheigea] - Thank you for contributing this!
I was reviewing it and would like to see the findbugs reported items that this fixes.
Seems like some are related to performance others are unused code and some other things.

I think that the change for the WebSSOResource actually points out a bug that the target audiences weren't being included as claims in the JWT token. I don't think we want to remove the initialization code but instead make sure that they are added to the resulting token. Will need to look closer at that.

Can you comment on your decision to make so many classes static? I am a bit skeptical of these sorts of "performance" changes as JVM optimizations make a lot of this irrelevant by inlining where possible, etc. That said, there may be an opportunity here - I just want to be careful to not make something static that will have unintended implications for extension or something like that.

Basically, what did you consider when evaluating the findbugs suggestions for those cases before changing them to static?

Thanks again for this work - it is great to clean up things like this!


was (Author: lmccay):
Hi [~coheigea] - Thank you for contributing this!
I was reviewing it and would like to see the findbugs reported items that this fixes.
Seems like some are related to performance others are unused code and some other things.

I think that the change for the WebSSOResource actually points out a bug that the target audiences weren't being included as claims in the JWT token. I don't think we want to remove the initialization code but instead make sure that they are added to the resulting token. Will need to look closer at that.

Can you comment on your decision to make so many classes static? I am a bit skeptical of these sorts of "performance" changes as JVM optimizations make a lot of this irrelevant by inlining where possible, etc. That said, there may be an opportunity here - I just want to be careful to not make something static that will have unintended implications for extension or something like that.

Basically, what did you consider when evaluating the findbugs suggestions for those cases before changing the to static?

Thanks again for this work - it is great to clean up things like this!

> Fix FindBugs "performance" issues
> ---------------------------------
>
>                 Key: KNOX-792
>                 URL: https://issues.apache.org/jira/browse/KNOX-792
>             Project: Apache Knox
>          Issue Type: Improvement
>            Reporter: Colm O hEigeartaigh
>            Assignee: Colm O hEigeartaigh
>            Priority: Trivial
>             Fix For: 0.11.0
>
>         Attachments: 0001-KNOX-702-Fix-FindBugs-performance-issues.patch
>
>
> This task is to fix the "performance" issues identified by findbugs.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)