Repository navigation
Resolve discriminators to codecs once per codec (plan item 3.3) - #4405
Merged
Merged
Conversation
Every entity decode looked up its discriminator through DiscriminatorLookup and the codec registry, even though the discriminator almost always names the codec's own type. MorphiaCodec.codecForDiscriminator returns the codec itself on a self-match and caches the codec for any other discriminator; EntityDecoder and LifecycleDecoder use it. decodeProperties skips the discriminator value instead of reading it into a string, and peekId reuses one DecoderContext and resolves its Object codec once per decoder.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No blocking defects were identified; the remaining lookup-count testing suggestion is optional.
1 open issue
What changed in this PR
Reduces repeated work in Morphia’s entity decoding while retaining the discriminator pre-scan.
Changes:
- Adds per-codec discriminator resolution caching and a self-match shortcut.
- Reuses
_iddecoding resources and skips already-processed discriminator values. - Adds codec-resolution identity tests for both mappers.
| File | Description |
|---|---|
| design/mapping-game-plan.md | Marks plan item 3.3 complete. |
| core/src/test/java/dev/morphia/test/mapping/codec/harness/CodecHarnessTest.java | Tests self-match and subtype codec identity. |
| core/src/main/java/dev/morphia/mapping/codec/pojo/MorphiaCodec.java | Adds discriminator codec caching. |
| core/src/main/java/dev/morphia/mapping/codec/pojo/LifecycleDecoder.java | Uses the shared discriminator resolver. |
| core/src/main/java/dev/morphia/mapping/codec/pojo/EntityDecoder.java | Reuses decoding resources and simplifies discriminator handling. |
Review effort: Balanced
A self-match leaves the cache empty, and repeated lookups of other discriminators leave one entry each. Identity checks alone couldn't tell a cached lookup from a repeated one, since the codec provider caches codecs by class too.
evanchooly
added a commit
that referenced
this pull request
Oct 22, 2026
… 3.4 + 3.5) (#4406) Plan items **3.4** and **3.5**. The plan notes that 3.4, a one-line fix, can ride along with any PR, so each has its own commit here. ## 3.4: `getProperty` does one lookup `EntityModel.getProperty` used `propertyModelsByMappedName.getOrDefault(name, propertyModelsByName.get(name))`. The default argument is evaluated eagerly, so it probed the second map on **every** call, including once per decoded field. It now checks the mapped-name map first and falls back only on a miss. The result is the same, since neither map holds null values. ## 3.5: memoize codec lookups by class The driver's `ProvidersCodecRegistry.get(Class)` allocates several objects on every call just to find a codec it has already cached: a `ChildCodecRegistry`, a `CodecCacheKey`, an `Optional` and a capturing lambda. Morphia looks codecs up per value, for array elements, map values, references, untyped properties and subtype values. Three changes: - **`MemoizingCodecRegistry`** (`@MorphiaInternal`) wraps the datastore's registry. A repeat `get(Class)` is a single `ConcurrentHashMap` hit. It uses `get` + `putIfAbsent` rather than `computeIfAbsent`, because building a codec can recurse into lookups. Failed lookups aren't cached. Lookups that take type arguments or a registry depend on their context, so they go straight to the wrapped registry. The driver caches one codec per class and returns the cached instance, and that's the instance memoized here, so results are unchanged. This also covers the driver's own per-operation lookups through `database.withCodecRegistry`. - **`MorphiaCodec.codecForSubtype(Class)`** caches the codec for each subtype value encoded through a codec. `EntityEncoder` and `LifecycleEncoder` use it instead of the codec's composite `fromRegistries(...)` registry, which is a driver registry and so allocates on every lookup. - **`ArrayCodec`** resolves its component codec once, instead of once per element. The cache field is `volatile` because the codec is shared across threads. `EntityDecoder`'s cached `peekId` codec from #4405 gets the same fix. ## Tests - `TestMemoizingCodecRegistry` uses a counting delegate to check three things: - repeated `get(String.class)` calls reach the delegate once and return the same codec; - a failing lookup isn't cached, so it fails each time; - contextual `get(Class, CodecRegistry)` lookups always reach the delegate. - The harness, creator and discriminator tests are unchanged and pass. The core and Kotlin suites pass with `-Dmorphia.mapper=critter` and with reflection: 1492 + 5 tests, 0 failures. ## JMH: `CodecBenchmark`, `master` (`05862218aea`) vs this branch Same machine as the baseline (i9-11900H, Temurin 17), `-prof gc`, the benchmark's own settings (3 forks × 5 × 1 s). Before and after ran back to back with nothing else running. Critter is the AOT variant. **reflection** | benchmark | model | before ns/op | after ns/op | Δ time | before B/op | after B/op | Δ alloc | |---|---|---:|---:|---:|---:|---:|---:| | decode | ARRAYS | 69,196 ± 483 | 68,390 ± 3,149 | -1.2% | 77,378 | 74,849 | -3.3% | | decode | COLLECTIONS | 9,091 ± 254 | 8,242 ± 64 | -9.3% | 16,824 | 16,712 | -0.7% | | decode | CONCRETE_COLLECTIONS | 3,015 ± 53 | 2,818 ± 89 | -6.5% | 6,272 | 6,128 | -2.3% | | decode | LAZY_REFERENCE | 3,260 ± 136 | 3,120 ± 124 | -4.3% | 7,755 | 7,293 | -5.9% | | decode | LIFECYCLE | 1,138 ± 27 | 1,063 ± 29 | -6.6% | 2,851 | 2,840 | -0.4% | | decode | NESTED | 3,015 ± 88 | 2,818 ± 33 | -6.5% | 5,776 | 5,656 | -2.1% | | decode | POLYMORPHIC | 5,461 ± 54 | 5,048 ± 64 | -7.6% | 9,656 | 9,528 | -1.3% | | decode | SIMPLE | 1,216 ± 14 | 1,102 ± 15 | -9.4% | 2,256 | 2,136 | -5.3% | | decodeInCursor | ARRAYS | 68,529 ± 509 | 67,378 ± 708 | -1.7% | 77,066 | 74,546 | -3.3% | | decodeInCursor | COLLECTIONS | 9,034 ± 126 | 8,347 ± 96 | -7.6% | 16,504 | 16,384 | -0.7% | | decodeInCursor | CONCRETE_COLLECTIONS | 3,071 ± 65 | 2,928 ± 72 | -4.7% | 5,968 | 5,848 | -2.0% | | decodeInCursor | LAZY_REFERENCE | 3,530 ± 42 | 3,260 ± 37 | -7.7% | 7,507 | 7,005 | -6.7% | | decodeInCursor | LIFECYCLE | 1,178 ± 38 | 1,120 ± 26 | -4.9% | 2,536 | 2,528 | -0.3% | | decodeInCursor | NESTED | 3,200 ± 269 | 2,819 ± 103 | -11.9% | 5,464 | 5,344 | -2.2% | | decodeInCursor | POLYMORPHIC | 5,565 ± 77 | 5,140 ± 109 | -7.6% | 9,336 | 9,216 | -1.3% | | decodeInCursor | SIMPLE | 1,185 ± 34 | 1,137 ± 38 | -4.0% | 1,944 | 1,824 | -6.2% | | encode | ARRAYS | 96,905 ± 1,853 | 97,915 ± 1,481 | +1.0% | 137,198 | 135,598 | -1.2% | | encode | COLLECTIONS | 7,774 ± 88 | 7,575 ± 74 | -2.6% | 7,400 | 7,336 | -0.9% | | encode | CONCRETE_COLLECTIONS | 2,060 ± 34 | 2,030 ± 31 | -1.5% | 3,136 | 3,104 | -1.0% | | encode | LAZY_REFERENCE | 1,806 ± 19 | 1,809 ± 78 | +0.2% | 4,083 | 3,885 | -4.8% | | encode | LIFECYCLE | 827 ± 8 | 802 ± 15 | -3.1% | 3,043 | 3,064 | +0.7% | | encode | NESTED | 2,281 ± 24 | 2,248 ± 44 | -1.4% | 1,856 | 1,856 | -0.0% | | encode | POLYMORPHIC | 4,293 ± 33 | 4,032 ± 71 | -6.1% | 6,024 | 4,584 | -23.9% | | encode | SIMPLE | 798 ± 12 | 777 ± 11 | -2.6% | 1,216 | 1,216 | -0.0% | **critter** | benchmark | model | before ns/op | after ns/op | Δ time | before B/op | after B/op | Δ alloc | |---|---|---:|---:|---:|---:|---:|---:| | decode | ARRAYS | 67,049 ± 741 | 66,715 ± 840 | -0.5% | 77,378 | 74,858 | -3.3% | | decode | COLLECTIONS | 8,845 ± 121 | 8,533 ± 102 | -3.5% | 16,824 | 16,688 | -0.8% | | decode | CONCRETE_COLLECTIONS | 3,005 ± 27 | 2,952 ± 83 | -1.7% | 6,272 | 6,128 | -2.3% | | decode | LAZY_REFERENCE | 3,323 ± 47 | 3,095 ± 96 | -6.9% | 7,752 | 7,227 | -6.8% | | decode | LIFECYCLE | 1,126 ± 12 | 1,056 ± 36 | -6.2% | 2,840 | 2,840 | -0.0% | | decode | NESTED | 3,222 ± 139 | 3,020 ± 52 | -6.3% | 5,752 | 5,656 | -1.7% | | decode | POLYMORPHIC | 5,697 ± 72 | 5,431 ± 115 | -4.7% | 9,664 | 9,528 | -1.4% | | decode | SIMPLE | 1,182 ± 74 | 1,135 ± 36 | -4.0% | 2,256 | 2,136 | -5.3% | | decodeInCursor | ARRAYS | 67,135 ± 608 | 67,103 ± 2,040 | -0.0% | 77,066 | 74,546 | -3.3% | | decodeInCursor | COLLECTIONS | 9,157 ± 145 | 8,776 ± 164 | -4.2% | 16,504 | 16,384 | -0.7% | | decodeInCursor | CONCRETE_COLLECTIONS | 3,109 ± 54 | 2,983 ± 36 | -4.1% | 5,976 | 5,840 | -2.3% | | decodeInCursor | LAZY_REFERENCE | 3,397 ± 91 | 3,114 ± 45 | -8.3% | 7,453 | 6,979 | -6.4% | | decodeInCursor | LIFECYCLE | 1,158 ± 21 | 1,136 ± 21 | -1.9% | 2,549 | 2,557 | +0.3% | | decodeInCursor | NESTED | 3,350 ± 153 | 2,969 ± 44 | -11.4% | 5,464 | 5,344 | -2.2% | | decodeInCursor | POLYMORPHIC | 5,770 ± 97 | 5,480 ± 101 | -5.0% | 9,336 | 9,216 | -1.3% | | decodeInCursor | SIMPLE | 1,221 ± 14 | 1,140 ± 58 | -6.7% | 1,944 | 1,824 | -6.2% | | encode | ARRAYS | 95,994 ± 1,289 | 98,008 ± 3,130 | +2.1% | 145,784 | 135,598 | -7.0% | | encode | COLLECTIONS | 7,650 ± 115 | 7,604 ± 69 | -0.6% | 7,336 | 7,336 | +0.0% | | encode | CONCRETE_COLLECTIONS | 2,016 ± 20 | 2,022 ± 30 | +0.3% | 3,104 | 3,147 | +1.4% | | encode | LAZY_REFERENCE | 1,697 ± 17 | 1,658 ± 16 | -2.3% | 4,072 | 3,832 | -5.9% | | encode | LIFECYCLE | 827 ± 19 | 784 ± 11 | -5.3% | 3,064 | 3,043 | -0.7% | | encode | NESTED | 2,348 ± 30 | 2,306 ± 15 | -1.8% | 1,856 | 1,856 | -0.0% | | encode | POLYMORPHIC | 4,290 ± 36 | 4,023 ± 58 | -6.2% | 5,640 | 4,200 | -25.5% | | encode | SIMPLE | 769 ± 27 | 735 ± 7 | -4.4% | 1,216 | 1,216 | -0.0% | **Reading it:** - **POLYMORPHIC encode allocates 1,440 B less (−24% / −25.5%) and runs about 6% faster.** That's the 12 subtype values per document no longer going through the driver's registry. The review estimated ~600 B per operation. - **Decode is 4–12% faster on most models, on both mappers.** That comes from 3.4's lookup per field, plus the memoized lookups. Decode allocation drops 120 B on SIMPLE, 2.5 KB on ARRAYS (the component codec) and ~500 B on LAZY_REFERENCE (reference lookups). - **No cell moved the wrong way beyond its error bars.** The critter `encode ARRAYS` allocation drop includes that cell's usual outlier `before` value. ## Decisions to review 1. **3.4 and 3.5 ship in one PR, as two commits.** The plan allows it for 3.4, and benchmarking a one-line change on its own would cost another hour of JMH. The table measures both together. 2. **The memoizing wrapper covers the whole datastore registry, not only Morphia's providers.** The driver's own lookups during operations benefit too. The wrapper never invents a codec: it caches what the driver returns, the driver itself keeps exactly one codec per class, and failed lookups aren't cached.
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
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.
Plan item 3.3 (review B2, step 1). Step 2, a single pass with no rewind, is plan item 3.17.
The problem
Every entity decode, both root and embedded, ran the discriminator pre-scan:
_t.DiscriminatorLookup.The decode then read
_tagain into a throwawayString. Separately,DecodeSession's_idpre-read (peekId) built a newDecoderContextand looked up theObjectcodec on every root decode.Change
MorphiaCodec.codecForDiscriminator(d)(@MorphiaInternal): returns the codec itself whendis its own model's discriminator. Otherwise it resolves the codec once and caches it per codec in aConcurrentHashMap. It usesget+putIfAbsentrather thancomputeIfAbsent, because building a codec can look up other codecs.EntityDecoder.getCodecFromDocumentandLifecycleDecoderboth use it.decodePropertiesskips the discriminator value (skipValue()) instead of reading it into a string.peekIdreuses a constantDecoderContextand resolves itsObjectcodec once per decoder.getCodecFromDocumentdrops its now-unusedregistryanddiscriminatorLookupparameters.EntityDecoderis@MorphiaInternal, and this was its only caller.Tests
CodecHarnessTest.codecForDiscriminatorResolvesEachTypeOnceruns under both mappers. A codec's own discriminator resolves to itself, and a subtype's discriminator (HarnessCirclethroughHarnessShape's codec) resolves to the subtype's codec, the same instance every time.TestCodecForDiscriminator(both mappers) checks the cache itself. A self-match leaves it empty, and repeated lookups of two other discriminators leave exactly two entries. It fails if the self-match shortcut is removed._t-late and subtype-in-declared-field fixtures. The core and Kotlin suites pass with-Dmorphia.mapper=critterand with reflection: 1487 + 5 tests, 0 failures.JMH:
CodecBenchmark,master(7d357ef7191) vs this branchSame machine as the baseline (i9-11900H, Temurin 17),
-prof gc, the benchmark's own settings (3 forks × 5 × 1 s). Before and after ran back to back with nothing else running. Critter is the AOT variant.reflection
critter
Reading it:
DecoderContextandObject-codec lookup inpeekId, plus the dropped throwaway_tstring. I didn't measure the parts separately.encode ARRAYSallocation drop (150,051 → 141,496 B) is the usual noise for that model, whosebeforevalue is an outlier against earlier runs.decode LAZY_REFERENCE, +4.1%. I reran it twice, alternating master and this branch. This branch was faster both times (3,329 vs 3,293 and 3,407 vs 3,328 ns/op) and allocated ~340 B less, so it was noise.Decisions to review
peekIdkeeps theObjectcodec rather than the id property's codec, even though the plan suggested the id property's codec. The pre-read id is the key the entity is registered under inDecodeSession. References look entities up by the id as stored in the reference: anObjectId, aString, or aDocumentfor a compound id. The id property's codec could produce a different value, such as a POJO for an embedded id or a converted type, and then the session lookup would miss and reference-cycle detection would stop working. Resolving theObjectcodec once gets the allocation win without that risk.getCodecFromDocument's signature changed. It'sprotectedon a@MorphiaInternalclass, and nothing outsideEntityDecodercalls it.