Cache URITemplates instead of creating them over and over resulting i…

[bcluap]

…n wasted CPU cycles and GC runs. The cache prevents having to compile java.util.regex.Pattern when the underlying templates seldom change as well as preventing other fairly heavy lifting each time a template is instantiated.

My load tests show approx 10% throughput increase on a basic JAX-RS service with 10 client threads in JMeter. My use case uses a lot of URITemplate instantiation due to calling javax.ws.rs.core.UriInfo.getMatchedURIs() on each sub-resource call.

I checked that URITemplate is thread safe and hence viable to be used in an instance cache without the need for additional locking.

[sberyozkin]

Thanks for this patch. It won't make it into 3.2.0/3.1.13 which are due very shortly.
Can you remove createExactTemplate and simply have createTemplate() updated given that with your patch createTemplate delegates to createExactTemplate ?
The other point: should the map be weakly hashed ?

[bcluap]

Thanks,

I added createExactTemplate due to direct calls to new URITemplate() from UriBuilderImpl which skipped the checks for null and a leading "/". I was not sure of the impact of forcing a leading "/" in the calls coming from UriBuilderImpl hence the added createExactTemplate which creates the template verbatim. If you think its ok to make it one function with the check to add the leading "/" then I will make that change.

In terms of weak references that's probably a good idea. I'll use Collections.synchronizedMap(new WeakHashMap...)

Let me know your thoughts and I'll amend accordingly

[sberyozkin]

thanks, let me get back to you early next week as I'm focusing on the issues which might still be fixed for 3.2.0... Re the weak map, may be it is not needed after all, not sure, realistically there's a limited set of concrete template values, may be keeping concurrent map is Ok for a start.

[andymc12]

Instead of clearing the cache when it reaches the max, could you use a LinkedHashMap (that overrides removeEldestEntry to make it an LRU cache)? That would remove the least-recently accessed entry, but still keep the rest of the cache intact. You would need to synchronize it too, but assuming you don't need to use a WeakHashMap, this might perform better for apps that use a large number of UriTemplates.

Thanks for finding this and making this change!

[sberyozkin]

Hi Andy, I'd rather avoid synchronzing and just clear the whole map now and then, the worst thing which would happen in this case is that the perf would return to the usual level which is there now, and it won't happen often. It's already done the same way for media types and providers

[sberyozkin]

By the way, the next step would be to try and create some optional non-static resource Method cache, JIRA exists for it :-), that would be quite a major improvement, just hard to do right

[sberyozkin]

Sorry for a delay, yes, when UriBuilderImpl does "new URiTemplate(...)" it is sensitive to the forward slash being added by default, so your code is fine.