perf: consolidate duplicate ObjectMapper instances into SerializerFactory#1774
Open
aws-kevinrickard wants to merge 2 commits intomainfrom
Open
perf: consolidate duplicate ObjectMapper instances into SerializerFactory#1774aws-kevinrickard wants to merge 2 commits intomainfrom
aws-kevinrickard wants to merge 2 commits intomainfrom
Conversation
…rFactory Replace 8 per-class ObjectMapper allocations with shared singletons in SerializerFactory. Each ObjectMapper carries its own type resolution, serializer, and deserializer caches that grow independently. Sharing instances eliminates duplicate cache warming and reduces RSS by several MB on memory-constrained devices. New shared mappers added to SerializerFactory: - getJsonObjectMapper() (default config) - getStrictJsonObjectMapper() (fail on unknown properties) - getCaseInsensitiveJsonObjectMapper() (case-insensitive properties) - getSortedJsonObjectMapper() (sorted properties and map entries)
aws-kevinrickard
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Binary incompatibility detected for commit f81828a. com.aws.greengrass.tes.CredentialRequestHandler is binary incompatible and is source incompatible because of FIELD_REMOVED Produced by binaryCompatability.py |
|
Unit Tests Coverage Report
Minimum allowed coverage is Generated by 🐒 cobertura-action against f81828a |
|
Integration Tests Coverage Report
Minimum allowed coverage is Generated by 🐒 cobertura-action against f81828a |
Replace deprecated ObjectMapper.enable(MapperFeature) with JsonMapper.builder().enable() as flagged by code scanning.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replace 8 per-class ObjectMapper allocations with shared singletons in SerializerFactory. Each ObjectMapper carries its own type resolution, serializer, and deserializer caches that grow independently. Sharing instances eliminates duplicate cache warming and reduces RSS by several MB on memory-constrained devices.
Changes
New shared mappers added to
SerializerFactory:getJsonObjectMapper()— default config (replaces instances in Coerce, MqttChunkedPayloadPublisher, MetricsAggregator, PubSubIPCEventStreamAgent)getStrictJsonObjectMapper()— fail on unknown properties (replaces CredentialRequestHandler)getCaseInsensitiveJsonObjectMapper()— case-insensitive properties (replaces AuthorizationPolicyParser)getSortedJsonObjectMapper()— sorted properties and map entries (replaces KernelConfigResolver)Also replaced a throwaway
new ObjectMapper()allocation in MetricsAggregator that was creating a fresh instance on every metrics upload just for debug logging.Testing
mvn compile— passesmvn test-compile— passes (all 173 test source files)Test installed onto an EC2 and profiled it, initial results are not that great, RSS for jackson went from about 10MB to about 9MB. This may need more testing or work to make it justified...
The caches we deduplicated are lazy — they grow as types are actually serialized/deserialized. Here's when the savings would be larger:
Long-running processes with diverse type usage — ObjectMapper caches grow over time as new types are encountered. On a real Greengrass device that's been running for hours/days, processing deployments, telemetry,
fleet status, IPC messages, and credential refreshes, each of those code paths warms its mapper's cache with different types. With 9 separate mappers, types used by multiple paths (like Map, List, String, basic
Jackson types) get cached independently in each. The longer it runs and the more code paths are exercised, the more duplication you eliminate.
After multiple deployments — Deployment processing touches the heaviest Jackson models (DeploymentDocument, ComponentRecipe, DeploymentPackageConfiguration, etc.). Each deployment cycle warms the serializer/
deserializer caches for these complex types. The old MetricsAggregator mapper and the DeploymentService mapper would both independently cache common base types.
Devices with many components — More components means more IPC traffic (auth policy parsing, pub/sub serialization, config store operations), more telemetry data, and more fleet status updates. All of these exercise
different mappers that now share caches.
The throwaway new ObjectMapper() in MetricsAggregator — That one created a fresh instance on every metrics aggregation cycle (every few minutes). Each one would allocate internal structures, serialize once, then
become garbage. Over time that's real GC pressure even if it doesn't show up in a point-in-time RSS measurement.