[
https://issues.apache.org/jira/browse/IGNITE-28356?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Anton Vinogradov updated IGNITE-28356:
--------------------------------------
Description:
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.
was:
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 item below is a problem found and the change that closes it.
A map whose values are always null.
All three producers of GridDistributedTxPrepareRequest#dhtVers put null as the
value, so the consumer's
`ver.getValue() == null || !equals` is always true. The field becomes
Collection<IgniteTxKey> dhtVerKeys,
dropping the value companion and 4 + 2N bytes.
A key travelling next to its own value.
GridNearTxPrepareResponse#ownedVals was a map plus two companions. The key
moves into the value as
KeyedVersionedValue, so the field becomes a single collection.
CacheVersionedValue already carries
cacheId, which now stops travelling twice: -(4 + 6N) bytes.
An array companion for an ordered set.
TxLocksRequest#txKeys and TxLocksResponse#txKeys kept a Set plus an array to
write it. writeCollection
and writeObjectArray emit identical bytes, so the companion is gone; the
runtime type stays HashSet and
the sender-side dedup is preserved.
A hand-written zip of two parallel fields.
GridNearGetRequest#keyMap was assembled by pairing keys and readersFlags by
index. The map stays and the
conversion moves into @Marshalled(keys = ..., values = ...).
A map copied entry by entry into an empty one.
GridDhtTxPrepareRequest#owned was drained into GridNearTxRemote with putAll
after the request was done
with it. ownedVersions now takes the map over, dropping a GridLeanMap and N
nodes per prepare.
Blob marshalling the codegen already expresses.
LazyServiceConfigurationMessage#affKey, GenericValueMessage#val,
QueryStartRequest#params and
QueryEntityMessage#dfltFieldValues 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.
Collection marshalling in an atomic request.
GridNearAtomicFullUpdateRequest#entryProcessors becomes @Marshalled; the field
is retyped to List<Object>
because the generated code calls add(Object). The operation() == TRANSFORM
gates stay on invokeArgs in all
three methods: dropping them only in marshal would fill invokeArgsBytes without
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, since the
codegen nulls the companion
after restoring it.
A job payload unmarshalled with the wrong class loader.
GridJobExecuteRequest had two hand-written methods because its five payload
fields are user classes that
only the deployment class loader can read, while GridIoManager#unmarshalPayload
would use the
configuration one. The fields become @Marshalled and the class becomes a
DeferredUnmarshalMessage, the
marker that exists for exactly this: 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.
Wire companions standing in for a conditional field. Covers IGNITE-28922, to be
closed as a duplicate.
CacheContinuousQueryEntry held keyWire/newValWire/oldValWire so that marshal()
could hide the data of a
filtered entry. The three pairs collapse and key/newVal/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 - handleEvent already discarded it on
arrival.
Wire layout is unchanged except where stated, and no path allocates more than
before, with one exception:
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.
> 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: 14.5h
> 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)
