[jira] [Assigned] (IGNITE-29109) TreeMap/TreeSet class descriptor is rebuilt on every binary write since 2.18.0 (regression of IGNITE-26061)

["Nikolay Izhikov (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/IGNITE-29109?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Nikolay Izhikov reassigned IGNITE-29109:
----------------------------------------

    Assignee: Nikolay Izhikov

> TreeMap/TreeSet class descriptor is rebuilt on every binary write since 2.18.0 (regression of IGNITE-26061)
> -----------------------------------------------------------------------------------------------------------
>
>                 Key: IGNITE-29109
>                 URL: https://issues.apache.org/jira/browse/IGNITE-29109
>             Project: Ignite
>          Issue Type: Bug
>            Reporter: Nikolay Izhikov
>            Assignee: Nikolay Izhikov
>            Priority: Major
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> h3. Summary
> Since 2.18.0 every binary write of a {{java.util.TreeMap}} or {{java.util.TreeSet}} rebuilds its {{BinaryClassDescriptor}} by reflection. An empty {{TreeMap}} costs ~25x more to marshal than in 2.17.0 (~6 µs vs ~0.26 µs); {{cache.put}} is ~10x slower and {{cache.get}} of a {{TreeMap}} value ~9x slower. {{HashMap}} and {{ArrayList}} are not affected. The compute closure wrappers {{GridClosureProcessor.C1/C1MLA/C2/C2MLA/C4/C4MLA}} take the same slow path. Reported on the dev list by a user; reproducer attached.
> h3. Measurements
> Reporter's reproducer (attached), one embedded server node, single thread, median of 5 rounds of 20 000 ops after warm-up, JDK 17, 12 CPUs, ns/op:
> ||Field value||2.17.0 toBinary||2.18.0 toBinary||2.17.0 put||2.18.0 put||2.17.0 get||2.18.0 get||
> |empty HashMap|395|521|1602|2669|1142|1673|
> |empty ArrayList|289|292|865|957|472|516|
> |empty TreeMap|261|*6221*|985|*9416*|532|*4728*|
> |empty TreeSet|364|*5722*|990|*8563*|587|556|
> |TreeMap, 12 entries|1098|*6956*|1873|*10208*|1139|*5335*|
> h3. Root cause
> Introduced by IGNITE-26061 (commit 06a557b2e4, PR #12230).
> # {{BinaryContext}} constructor now calls {{registerBinarilizableSystemClass}} for {{BinaryTreeMap}}, {{TreeMap}} and {{TreeSet}}; {{GridClosureProcessor.onBinaryContextCreated}} does the same for the six closure wrappers.
> # {{registerBinarilizableSystemClass}} puts {{systemClassDescriptor(...)}} into {{descByCls}}. That descriptor is created with {{registered = false}}.
> # On every write {{registerClass}} sees {{!desc.registered()}} and calls {{registerDescriptor}}, which builds a registered copy with {{desc.makeRegistered()}} (full {{BinaryClassDescriptor}} constructor with reflective look-ups) and offers it with {{descByCls.putIfAbsent(...)}}.
> # The unregistered entry is already present, so {{putIfAbsent}} returns it and the registered copy is discarded. The next write repeats the work.
> Before IGNITE-26061 these classes were listed by name in {{BINARYLIZABLE_SYS_CLSS}} and nothing was in {{descByCls}} before the first write, so the first {{putIfAbsent}} cached the registered descriptor.
> h3. Fix
> Cache the descriptor in the registered state:
> {code:java}
> public void registerBinarilizableSystemClass(Class<?> cls) {
>     String clsName = cls.getName();
>     descByCls.put(cls, systemClassDescriptor(cls, clsName, new BinaryReflectiveSerializer()).makeRegistered());
> }
> {code}
> With the fix the reproducer shows empty {{TreeMap}} at 453 ns toBinary / 1119 ns put / 580 ns get, i.e. back at 2.17.0 level. A regression test is added to {{BinaryContextPredefinedTypesTest}} asserting that the cached descriptors of these system classes are registered and that {{registerClass}} returns the cached instance.
> h3. Reproducer
> {{treemap-descriptor-repro.zip}} (attached): {{mvn -q compile exec:exec}} runs against 2.18.0, {{-Dignite.version=2.17.0}} for comparison. See its README.md.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)