awslabs / awslabs/payload-offloading-java-common-lib-for-aws
PayloadS3Pointer inefficiently uses Jackson's ObjectMapper
- 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
Assessment
This issue has not been assessed yet.