Excessive use of GC

[greenwoodma]

Currently the code does an explicit GC both before and after loading the gazetteer so that it can report on memory consumption. It does this even when duplicating a PR in which no extra memory is allocated. We've seen each GC take as much as 5s, so when using this on cloud or GCP it can add quite an overhead: 40s for 4 threads is just GC. On cloud this is possibly what's causing the prometheus warnings as the full load time is high.

The suggestion is to add a new debug or profiling option to the PR, so that in normal usage we don't report the memory consumption and don't explicitly run the GC.

[ianroberts]

I've confirmed that it is the System.gc() calls that are causing the slowdown, by profiling the same app with -XX:+DisableExplicitGC (which makes System.gc() a no-op).

[greenwoodma]

@ianroberts I'm part way there with a new profile option which deals with the explicit GC in the PR classes, but what do we think about the one here

gateplugin-StringAnnotation/src/main/java/com/jpetrak/gate/stringannotation/extendedgazetteer/trie/StoreStates.java

Line 95 in 58f5ad6

System.gc();

Annoyingly it's not in the PR so would need to pass an option through to turn it on/off. I'm tempted just to remove it completely as that code gets run between the other two explicit GC calls so from the point of view of seeing the increase in heap it shouldn't make any difference, but what do you think?

[ianroberts]

This particular one I guess is slightly less crucial as it's not on the critical path when duplicating an existing PR, only when first loading a new one, but yeah, either take it out completely or add some sort of static configuration like only doing the GCs if System.getProperty("com.jpetrak.gate.stringannotation.profile") != null or something. Actually that might be a better way to approach the whole thing, rather than making it a PR param - you can turn the GCs on if you want them in development, but in normal use you'd almost always want them turned off anyway.