RFR: 8037325: Class.getConstructor() performance regression

[Claes Redestad]

Hi Christoph,

thanks for looking at this!

Extracting methodName was part cleanup, part meant to
outline some rarely executed error handling code, which
can help a JIT focus on the critical path.

I'd prefer not to micro-optimize argumentTypesToString
(and would likely look at other things than StringJoiner if it
was performance sensitive).

Thanks!

/Claes

On 01/13/2017 01:29 PM, Christoph Dreis wrote:
> Hey,
>
> your extraction of methodName() made me aware of another small thing. What about rewriting argumentTypesToString() to avoid creating the StringJoiner if not needed?
>
>      private static String argumentTypesToString(Class<?>[] argTypes) {
>          if (argTypes != null) {
>              StringJoiner sj = new StringJoiner(", ", "(", ")");
>              for (int i = 0; i < argTypes.length; i++) {
>                  Class<?> c = argTypes[i];
>                  sj.add((c == null) ? "null" : c.getName());
>              }
>              return sj.toString();
>          }
>          return "()";
>      }
>
> Should have a very minor impact though. Apart from that looks good to me.
>
> Cheers,
> Christoph
>
> -----Original Message-----
> From: core-libs-dev [mailto:core-libs-dev-bounces at openjdk.java.net] On Behalf Of Claes Redestad
> Sent: Thursday, January 12, 2017 3:48 PM
> To: core-libs-dev Libs <core-libs-dev at openjdk.java.net>; Security Dev OpenJDK <security-dev at openjdk.java.net>
> Subject: RFR: 8037325: Class.getConstructor() performance regression
>
> Hi,
>
> please review this fix to various performance regressions observed
> as the security model has evolved over the years.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8037325
> Webrev: http://cr.openjdk.java.net/~redestad/8037325/webrev.01
>
> - For cases where a SecurityManager is not installed, this improves
> performance by avoiding calls to Reflection.getCallerClass, which
> can be very expensive when not inlined. Regrettably this adds some
> boilerplate.
>
> - For cases where a SecurityManager is installed, this improves
> performance slightly by avoiding repeated calls to
> System.getSecurityManager (which does volatile read).
>
> - Use of Class.getPackageName when appropriate reduce the
> number of allocations done.
>
> - Places where doPrivileged calls were used to bypass access checking
> when calling methods on Class can safely be replaced by calling
> corresponding private methods directly if care is taken to copy the
> end result as appropriate.
>
> - Finally, by using the recently used
> ReflectionFactory.getExecutableSharedParameterTypes we also get rid of
> some unnecessary copying.
>
> Thanks!
>
> /Claes
>