[
https://issues.apache.org/jira/browse/IGNITE-28356?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18100076#comment-18100076
]
Anton Vinogradov commented on IGNITE-28356:
-------------------------------------------
[Ignite PR Checker|https://ignite-pr-checker.is-a.dev/?pr=13408] verdict for PR
13408 · RunAll build [9236765|https://ci2.ignite.apache.org/build/9236765] ·
147 suites ran, 0 reused
(x) *2 blocker(s) in 2 suite(s):*
- Queries 5: {{org.apache.ignite.testsuites.IgniteBinaryCacheQueryTestSuite5:
org.apache.ignite.internal.processors.cache.index.DynamicEnableIndexingBasicSelfTest.testEnableDynamicIndexing[hasNear=false,nodeIdx=3,cacheMode=PARTITIONED,atomicityMode=ATOMIC]}}
- Cache 2: {{org.apache.ignite.testsuites.IgniteCacheTestSuite2:
org.apache.ignite.internal.processors.cache.distributed.near.GridNearTxPrepareDestroyedCacheTest.testPrepareOfDestroyedCacheAnsweredWithErrorResponse}}
⏳ _Auto re-run *#1* in progress — 2 blocker suite(s) re-queued (attempt 1/2).
This comment updates when they settle._
> Get rid of redundant wire companion fields and hand-written marshalling in
> messages
> -----------------------------------------------------------------------------------
>
> Key: IGNITE-28356
> URL: https://issues.apache.org/jira/browse/IGNITE-28356
> Project: Ignite
> Issue Type: Task
> Reporter: Ilya Shishkov
> Assignee: Anton Vinogradov
> Priority: Minor
> Labels: IEP-132, ise
> Fix For: 2.19
>
> Time Spent: 14h 50m
> Remaining Estimate: 0h
>
> The message codegen (IEP-132) can marshal a logical field into its wire
> companion by itself, yet many
> messages still keep both the field and a hand-written
> \{{marshal()}}/\{{unmarshal()}} pair that fills the
> companion. Some of those pairs carry no logic at all, and some of the
> companions describe data that is
> never sent. Each section below is a problem found and the change that closes
> it.
> h3. A map whose values are always null
> {\{GridDistributedTxPrepareRequest#dhtVers}} was a map plus two companions.
> All three producers put
> {\{null}} as the value, so the consumer's \{{ver.getValue() == null ||
> !equals}} is always true.
> *Fix:* the field becomes \{{Collection<IgniteTxKey> dhtVerKeys}}. The value
> companion is gone and the
> request is \{{4 + 2N}} bytes shorter.
> h3. A key travelling next to its own value
> {\{GridNearTxPrepareResponse#ownedVals}} was a map plus two companions, and
> \{{CacheVersionedValue}}
> already carries \{{cacheId}}, so the cache id travelled twice.
> *Fix:* the key moves into the value as the new \{{KeyedVersionedValue}}, and
> the field becomes a single
> collection. The response is \{{4 + 6N}} bytes shorter.
> h3. An array companion for an ordered set
> {\{TxLocksRequest#txKeys}} and \{{TxLocksResponse#txKeys}} kept a \{{Set}}
> plus an array to write it, even
> though \{{writeCollection}} and \{{writeObjectArray}} emit identical bytes.
> *Fix:* the companions are gone. The runtime type stays \{{HashSet}}, so the
> sender-side dedup is
> preserved and the wire format does not move.
> h3. A hand-written zip of two parallel fields
> {\{GridNearGetRequest#keyMap}} was assembled by pairing \{{keys}} and
> \{{readersFlags}} by index, and taken
> apart again on the way out.
> *Fix:* the map stays where the code reads better, and the conversion moves
> into
> {\{@Marshalled(keys = ..., values = ...)}}.
> h3. A map copied entry by entry into an empty one
> {\{GridDhtTxPrepareRequest#owned}} was drained into \{{GridNearTxRemote}}
> with \{{putAll}}, although the
> request is done with it by then.
> *Fix:* \{{ownedVersions}} takes the map over instead of copying it, which
> drops a \{{GridLeanMap}} and
> {\{N}} nodes per prepare.
> h3. Blob marshalling the codegen already expresses
> {\{LazyServiceConfigurationMessage#affKey}}, \{{GenericValueMessage#val}},
> \{{QueryStartRequest#params}}
> and \{{QueryEntityMessage#dfltFieldValues}} each had a hand-written pair
> doing nothing but
> {\{U.marshal}}/\{{U.unmarshal}}.
> *Fix:* all four become \{{@Marshalled}}; the generated code matches the
> hand-written one call for call.
> {\{QueryEntityMessage}} keeps its \{{!F.isEmpty}} gate, moved into the
> constructor, because
> {\{QueryEntity}} initializes the defaults with an empty map while the
> generated gate is \{{!= null}}.
> h3. Collection marshalling in an atomic request
> {\{GridNearAtomicFullUpdateRequest#entryProcessors}} was marshalled by hand,
> so the request carried both
> the objects and their bytes until it was collected.
> *Fix:* the field becomes \{{@Marshalled}}, retyped to \{{List<Object>}}
> because the generated code calls
> {\{add(Object)}}. Two points worth reviewing:
> * the \{{operation() == TRANSFORM}} gates stay on \{{invokeArgs}} in all
> three methods. Dropping them only
> in \{{marshal}} would fill \{{invokeArgsBytes}} without calling
> \{{deployInvokeArguments}}, and a receiver
> with peer class loading on would get a \{{NoClassDefFoundError}} instead of
> today's silent degradation;
> * the \{{deploy()}} gate becomes \{{entryProcessors != null}}, because the
> codegen nulls the companion
> after restoring it, which makes the old \{{entryProcessorsBytes == null}}
> wording false in meaning.
> h3. A job payload unmarshalled with the wrong class loader
> {\{GridJobExecuteRequest}} kept two hand-written methods because its five
> payload fields are user classes
> that only the deployment class loader can read, while
> \{{GridIoManager#unmarshalPayload}} would hand them
> the configuration one.
> *Fix:* the fields become \{{@Marshalled}} and the class becomes a
> \{{DeferredUnmarshalMessage}}, the marker
> that exists for this case. \{{GridJobProcessor}} unmarshals the request
> itself, with the same loader as
> before. A job of a continuous task also stops sending its siblings: the
> receiver discarded them, and
> {\{GridJobSessionImpl#getJobSiblings}} requests them from the task node when
> they are missing.
> h3. Wire companions standing in for a conditional field
> Covers IGNITE-28922, to be closed as a duplicate.
> {\{CacheContinuousQueryEntry}} held \{{keyWire}}, \{{newValWire}} and
> \{{oldValWire}} purely so that
> {\{marshal()}} could hide the data of a filtered entry at serialization time.
> *Fix:* the three pairs collapse and \{{key}}, \{{newVal}} and \{{oldVal}} go
> back to plain \{{@Order}}. The
> invariant becomes structural instead: a filtered entry gives its data up at
> {\{CacheContinuousQueryEventBuffer#processEntry}}, the only door into the
> buffer, so nothing downstream
> has to remember it. Two copies this makes redundant are removed, and a
> filtered \{{EXPIRED}} entry with
> {\{updateCntr == -1}} is no longer sent at all, since \{{handleEvent}}
> already discarded it on arrival.
> h3. Cost
> The wire format is unchanged except where a section states a reduction, and
> no path allocates more than
> before, with one exception worth naming: a filtered entry taking the
> out-of-range counter path in the
> continuous query buffer now costs one object, where the data used to be
> masked at serialization time
> instead.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
