Closed sunpei1 closed 1 year ago
Great find! Although I am surprised that getSelectionFormula() is called more than once in Keep, it's a good idea to cache the value. @jesse-gallagher what do you think? Is caching the memory structure size safe or are the any cases where this value changes dynamically (e.g. for the db design)?
I think it should be fine - while an individual instance may have a different size, that method only deals with the Class
anyway.
However, I think it'd be good to replace the HashMap
with a ConcurrentHashMap
and use the computeIfAbsent
method to calculate if necessary. That will be both more explicit in wording and also thread-safe. That'd be the same pattern as in my variant JNXServiceFinder.
@jesse-gallagher good suggestion, I'll replace it.
@jesse-gallagher updated with concurrentHashMap, please review:)
Here's the profiling results with this patch, the formula branch disappear from the fireflame graph:
@jesse-gallagher is there any more comments on this PR?
Most likely not - I'm out this week and last, though, so I expect that I'll merge it in next week.
When testing Keep /lists endpoint, we found some hotspot area in jnx code:
DominoCollectionColumn.getFomula()
is expensive, since it triggers fomula get and decompile everytime.DominoCollectionColumn.getItemName()
contributes around 1/7 getFormula()'s cost.MemoryStructureUtil.sizeOf()
consumes around 4% of all the time. (After accelerating formula and itemName part, sizeOf becomes more hot.) Seems this cost can be saved by avoiding recursive calucation in StructureMap.size().This PR use cache to avoid these hotspots. Together with the previous findRequiredService fix, all these changes can double the /lists endpoint's throughput.