[
https://issues.apache.org/jira/browse/IGNITE-28520?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Anton Vinogradov reassigned IGNITE-28520:
-----------------------------------------
Assignee: Anton Vinogradov (was: Alex Abashev)
> Auto-generate prepareMarshalCacheObjects to replace hand-written CO traversal
> in GridCacheMessage subclasses
> ------------------------------------------------------------------------------------------------------------
>
> Key: IGNITE-28520
> URL: https://issues.apache.org/jira/browse/IGNITE-28520
> Project: Ignite
> Issue Type: Task
> Reporter: Alex Abashev
> Assignee: Anton Vinogradov
> Priority: Minor
> Labels: IEP-132, ise
> Fix For: 2.19
>
> Time Spent: 4h 40m
> Remaining Estimate: 0h
>
> h2. Summary
> The CacheObject pre-marshal pass already runs on the user thread —
> {{GridCacheIoManager.onSend}} has been calling {{msg.prepareMarshal(cctx)}}
> on the sender thread since 2014 (commit by Anton Irinev, before
> {{ignite-63}}). This ticket is *not* about moving work off the NIO worker; it
> is about replacing the per-class hand-written CO traversal with a generated,
> auto-invoked equivalent.
> Today every {{GridCacheMessage}} subclass that carries {{CacheObject}} fields
> has a hand-written {{prepareMarshal(GridCacheSharedContext)}} override that
> walks those fields and calls {{co.prepareMarshal(ctx)}} on each. Adding a new
> {{@Order CO}} field to such a class requires a matching update of the
> override. Forgetting that update silently moves that field's
> {{Marshaller.marshal}} call onto the NIO writer (it runs lazily inside
> {{writeTo}} when {{valBytes == null}}), which is the regression mode this
> ticket exists to prevent.
> After this ticket lands, the same traversal is generated by APT and
> auto-invoked from the framework — so coverage is structural rather than
> maintained by hand.
> h2. What changes
> * {{MessageSerializer}} interface gets a new {{default void
> prepareMarshalCacheObjects(M msg, CacheObjectValueContext ctx)}} (no-op
> default).
> * {{MessageSerializerGenerator}} (APT) emits this method on every concrete
> serializer with at least one traversable {{@Order}} field; recurses into
> nested concrete {{Message}} types via per-class {{*_SER}} static fields.
> Recursion is blocked only for {{GridCacheIdMessage}} subclasses (top-level
> routable messages) — helper messages with {{@Order int cacheId}}
> ({{GridCacheReturn}}, {{IgniteTxKey}}, {{IgniteTxEntry}},
> {{GridCacheEntryInfo}}, {{UpdateErrors}}) are recursable.
> * {{GridCacheIoManager.onSend}} adds
> {{prepareMarshalGeneratedCacheObjects(msg)}} after the existing
> {{msg.prepareMarshal(cctx)}}. Helper is gated on {{msg instanceof
> GridCacheIdMessage}} and resolves {{cacheObjectContext}} from
> {{((GridCacheIdMessage)msg).cacheId()}}. Both calls run on the user thread,
> both are idempotent for any given {{CacheObject}} ({{if (valBytes == null)}}).
> * Hand-written {{prepareMarshal(GridCacheSharedContext)}} overrides are
> removed from {{GridCacheMessage}} subclasses where the body was a pure
> {{@Order}}-CO traversal that the generator now performs:
> ** {{GridCacheTtlUpdateRequest}}, {{GridDhtForceKeysRequest}},
> {{GridNearUnlockRequest}}, {{GridDhtUnlockRequest}},
> {{GridDistributedLockRequest}}, {{GridDistributedLockResponse}},
> {{GridDhtAtomicUpdateResponse}}, {{GridDhtAtomicNearResponse}},
> {{GridNearGetRequest}}.
> ** Also dropped: {{ret.prepareMarshal(cctx)}} call inside
> {{GridNearAtomicUpdateResponse.prepareMarshal}} (covered by nested-message
> recursion in the generated serializer).
> * Generator hygiene aligned with Ignite coding guidelines: {{private static
> final}} modifier order, no trailing space after {{/**}} in class javadoc, no
> trailing newline at EOF, empty line between top-level {{if}}-blocks in
> {{prepareMarshalCacheObjects}}, empty line between map keys/values traversal
> blocks.
> h2. Why it is worth doing
> * *Structural coverage.* New {{@Order CO}} fields are picked up
> automatically; no risk of a missed override.
> * *Less boilerplate.* Phase 2 cleanup deletes ~9 hand-written methods that
> were doing the same loop the generator now writes.
> * *One source of truth* for "which fields participate in pre-marshal
> traversal" — the {{@Order}} annotations.
> * *No behavioural change* for messages whose hand-written override stays —
> the auto-invoke is a no-op on already-marshalled {{CacheObject}}s thanks to
> the {{valBytes == null}} guard inside {{CacheObjectImpl.prepareMarshal}}.
> h2. Out of scope
> * Receive-side counterpart ({{finishUnmarshalCacheObjects}}) — split into a
> separate ticket (IGNITE-YYYYY).
> * Extending the auto-invoke to {{GridCacheMessage}} subclasses that are not
> {{GridCacheIdMessage}} (e.g. {{GridDhtTxFinishResponse}}), or to nested
> messages whose embedded {{cacheId}} differs from the outer's (e.g.
> {{GridNearTxPrepareResponse}}'s {{retVal.cacheId()}}) — split into a separate
> ticket (IGNITE-ZZZZZ). Until that lands, {{GridCacheReturn}} keeps its
> {{static SER}} field and {{prepareMarshal(GridCacheContext)}} bridge for the
> two responses that fall outside the auto-invoke gate.
> h2. Acceptance criteria
> * {{GridCacheIoManager.onSend}} calls
> {{prepareMarshalGeneratedCacheObjects(msg)}} for every {{GridCacheIdMessage}}
> subclass with a non-{{null}} {{cacheObjectContext}}.
> * {{MessageProcessorTest}} green; fixture serializers byte-identical with
> generator output.
> * Targeted suites green: {{GridCacheReturnValueTransferSelfTest}},
> {{IgniteCacheEntryProcessorCallTest}},
> {{GridCachePartitionedTxMultiThreadedSelfTest}},
> {{GridCacheRebalancingSyncSelfTest}} (force-keys path),
> {{GridCachePutAllFailoverSelfTest}} (atomic update / unlock path),
> {{IgniteCacheQueryMultiThreadedSelfTest}} (query/eviction path).
> * Possible-blockers list from the TC bot for the PR contains no real
> regressions (flakes only).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
