awslabs / awslabs/payload-offloading-java-common-lib-for-aws

PayloadS3Pointer inefficiently uses Jackson's ObjectMapper

Open
#51 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
34
Forks
17
PR merge metrics
No merged PRs in 30d

Description

Ref:
- [PayloadS3Pointer](https://github.com/awslabs/payload-offloading-java-common-lib-for-aws/blob/df2156bfbaa40b272369d6ffa1a6d37cace1bc67/src/main/java/software/amazon/payloadoffloading/PayloadS3Pointer.java#L35)
- [JsonDataConverter](https://github.com/awslabs/payload-offloading-java-common-lib-for-aws/blob/master/src/main/java/software/amazon/payloadoffloading/JsonDataConverter.java#L19)
- https://github.com/FasterXML/jackson-docs/wiki/Presentation:-Jackson-Performance#basics-things-you-should-do-anyway

> Reuse heavy-weight objects: ObjectMapper (data-binding) and JsonFactory (streaming API)
To a lesser degree, you may also want to reuse ObjectReader and ObjectWriter instances -- this is just some icing on the cake, but they are fully thread-safe and reusable

Short version:
1. PayloadS3Pointer instantiates a new JsonDataConverter for every `toJson()` invocation.
2. JsonDataConverter instantiates a new ObjectMapper for every invocation.

When used via [a S3BackedPayloadStore](https://github.com/awslabs/payload-offloading-java-common-lib-for-aws/blob/df2156bfbaa40b272369d6ffa1a6d37cace1bc67/src/main/java/software/amazon/payloadoffloading/S3BackedPayloadStore.java) in the [SQS Extended Client](https://github.com/awslabs/amazon-sqs-java-extended-client-lib/blob/50a9248d40ef2bbfe88ff7363b6fff1362b32c61/src/main/java/com/amazon/sqs/javamessaging/AmazonSQSExtendedClient.java#L988) and sending a ton of SQS messages, this can add up to quite the performance hit.

If you flipped this on its head and
1. Had the PayloadStore contain an ObjectMapper
2. Got rid of the `toJson()` method
3. Serialized the PayloadS3Pointer directly in the PayloadStore
- Probably via an abstraction containing the ObjectMapper so you could encapsulate the Exception handling nicely
- See also: https://testing.googleblog.com/2020/12/testing-on-toilet-separation-of.html

Then you could see some performance benefit by having one ObjectMapper per-PayloadStore. But, making a static, hidden, shared ObjectMapper throughout the entire library would see the biggest benefits. Again, probably contained in a Singleton wrapper class that is used and/or injected everywhere.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.