Thanks Colvin for the analysis. Very helpful! I added the one here: org.apache.solr.packagemanager.PackageUtils.getMapper() Now, that you mentioned, I'll change it to reuse it. However, this particular one is not performance critical as it is called from the SolrCLI (when "bin/solr package" support is used), and its life is as long as a single command.
@Jan Høydahl can you please review the AuditLogger one? @Noble Paul നോബിള് नोब्ळ् can you please review the SolrJacksonAnnotationInspector one? On Thu, Dec 19, 2019 at 11:00 PM Colvin Cowie <[email protected]> wrote: > > Hi, > > Looking through the source code to check up on a vulnerability in Jackson, I > saw that there's a couple of places on master where new ObjectMappers are > being created and not reused. > > I've found through experience that not reusing ObjectMappers (especially in > loops) can have a very detrimental impact on performance. Reuse is > recommended: > https://github.com/FasterXML/jackson-docs/wiki/Presentation:-Jackson-Performance#basics-things-you-should-do-anyway > - "1. Reuse heavy-weight objects: ObjectMapper (data-binding)" > ObjectMapper is thread safe once configured. > > The older uses just create them statically: > > org.apache.solr.analytics.AnalyticsRequestParser > org.apache.solr.prometheus.scraper.SolrScraper > > The new ones I see currently are in > > org.apache.solr.security.AuditLoggerPlugin.JSONAuditEventFormatter.formatEvent(AuditEvent) > org.apache.solr.packagemanager.PackageUtils.getMapper() > org.apache.solr.util.SolrJacksonAnnotationInspector.createObjectMapper() > > I don't know whether these uses are necessarily going to cause performance > issues or not, but it's generally best not to new them up inline. > > Colvin > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
