Startup time improvement #4
Conversation
Postremus
force-pushed
the
startup-time-improvement
branch
from
8c319d7 to
87a6faa
Compare
|
I will refrain from further tinkering on this problem (though it is fun). I listed the essential commits in the issue description. The last two (8d929bb and 2d9d3a5) combined save about another 20ms. |
| @@ -525,8 +525,7 @@ private static Method getGetterOrNull(Class containerClass, String propertyName) | |||
| // try "get" | |||
| if ( methodName.startsWith( "get" ) ) { | |||
| final String stemName = methodName.substring( 3 ); | |||
| final String decapitalizedStemName = Introspector.decapitalize( stemName ); | |||
| if ( stemName.equals( propertyName ) || decapitalizedStemName.equals( propertyName ) ) { | |||
| if ( stemName.equals( propertyName ) || Introspector.decapitalize( stemName ).equals( propertyName ) ) { | |||
| @@ -49,6 +51,9 @@ | |||
| private static final Method OBJECT_EQUALS; | |||
| private static final Method OBJECT_HASHCODE; | |||
|
|
|||
| private static final Map<Class<?>, Method[]> METHODS_BY_CLASS = new HashMap<>(); | |||
| private static final Map<Class<?>, Field[]> FIELDS_BY_CLASS = new HashMap<>(); | |||
There was a problem hiding this comment.
So this is not possible for us to do, unfortunately. We can't have the Hibernate classloader hold a "hard reference" to a class instance of the application: many application servers woul start leaking memory, as the Hibernate module is long-lived while applications (and user classes) are often undeployed, redeployed, etc.. sometimes many times.
What we can do is to introuce a new class type IntrospectedClass (or anything) to store together: the user class, its methods, its fields. The methods array and the fields array could be lazily initialized.
Then you refactor this class to consistently use that new IntropectedClass instead of the current Class.
If I'm unclear let me know, I'm happy to show you what I mean :)
This approach would also benefit from not using excessive map inserts/lookups.
There was a problem hiding this comment.
Hi @Sanne,
sorry for the late reply, had to focus my creativity on real (as in paid) work for the last few weeks.
I gave your suggestion a shot in Postremus@7d02e58.
I quickly noticed though that, as far as I understand, this won't work. IntrospectedClass is a clean idea to solve the problem of hard references on class instances.
However, the current structure of ORM does not lend itself to this concept.
Using IntrospectedClass would also mean using it in all call sites and their call sites in turn. This would result in way to many changes, and also needs changes in spi interfaces (e.g. PropertyAccess, Getter, Setter).
Otherwise IntrospectedClass is always for each field access, rendering it useless.
I have switched the commit order around abit (I love interactive rebasing) - this commit is now the last. I am okay with simply dropping it.
ORM 6 is dropping in the next few weeks anyway, where work could be started on using jandex instead of reflection.
Postremus
force-pushed
the
startup-time-improvement
branch
from
2d9d3a5 to
380634b
Compare
Postremus
force-pushed
the
startup-time-improvement
branch
from
380634b to
3c05d93
Compare
|
FTR, there is one related test failure, which I will have to look into. I also dropped the caching commit for the reasons mentioned above. |
This saves on throwing exceptions everytime a field was not found, for the cost of copying the fields arrays on each invocation. This is still magnitudes faster though.
Postremus
force-pushed
the
startup-time-improvement
branch
2 times, most recently
from
da70482 to
e7f01ff
Compare
|
All tests are passing now. |
|
@Sanne friendly ping. :) |
Relates to https://quarkusio.zulipchat.com/#narrow/stream/187030-users/topic/startup.20.2F.20reloading.20performance
d87ddc0
Avoids a duplicate getParameterTypes() call, just for good measure.
37b219c
Saves about 700ms.
8145598
Saves another 1s. This replaces a getDeclaredField call by getDeclaredFields and an iteration. Exceptions are apperantly more costly than creating lots of arrays copies.
8c319d7
Saves another 300ms. This is the commit you worried about, implementing a simple cache for the class instances declared methods and fields.