Jackson ObjectMapper reuse

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Jackson ObjectMapper reuse

Colvin Cowie
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:
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

Reply | Threaded
Open this post in threaded view
|

Re: Jackson ObjectMapper reuse

Ishan Chattopadhyaya
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
<[hidden email]> 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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Jackson ObjectMapper reuse

Noble Paul നോബിള്‍  नोब्ळ्
SolrJacksonAnnotationInspector

is OK, because it is created and given to the original caller who
creates it. The caller is supposed to cache and reuse it


On Fri, Dec 20, 2019 at 4:41 AM Ishan Chattopadhyaya
<[hidden email]> wrote:

>
> 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
> <[hidden email]> 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
> >



--
-----------------------------------------------------
Noble Paul

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Jackson ObjectMapper reuse

Jan Høydahl / Cominvent
AuditLoggerPlugin: https://issues.apache.org/jira/browse/SOLR-14129

Jan

20. des. 2019 kl. 01:28 skrev Noble Paul <[hidden email]>:

SolrJacksonAnnotationInspector

is OK, because it is created and given to the original caller who
creates it. The caller is supposed to cache and reuse it


On Fri, Dec 20, 2019 at 4:41 AM Ishan Chattopadhyaya
<[hidden email]> wrote:

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
<[hidden email]> 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




--
-----------------------------------------------------
Noble Paul