xiangfu0 commented on code in PR #19795:
URL: https://github.com/apache/pinot/pull/19795#discussion_r4252236506
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/customobject/tdigest/SerializedTDigest.java:
##########
@@ -16,32 +16,34 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.pinot.segment.local.customobject;
+package org.apache.pinot.segment.local.customobject.tdigest;
-import com.tdunning.math.stats.TDigest;
-import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
import org.apache.pinot.spi.utils.BytesUtils;
import static com.google.common.base.Preconditions.checkArgument;
-/// Serialized and comparable version of TDigest. Compares TDigest for a
specific percentile value.
+/// Immutable serialized result and comparison key for a specific percentile
value.
+/// Uses Double.compare ordering: NaN quantiles sort last ascending and first
descending, ahead of positive infinity.
public class SerializedTDigest implements Comparable<SerializedTDigest> {
private final double _percentile;
- private final TDigest _tDigest;
+ private final byte[] _serializedBytes;
+ private final double _quantile;
public SerializedTDigest(TDigest tDigest, double percentile) {
- _tDigest = tDigest;
_percentile = percentile / 100.0;
+ // Finalize once before obtaining the sort key; serialization also owns
historical byte-retention policy.
+ _serializedBytes = tDigest.serialize();
+ _quantile =
PercentileTDigestAccumulator.fromBytes(_serializedBytes).quantile(_percentile);
Review Comment:
Fixed in 32f388a46e: the comparison key now interpolates directly over the
finalized encoding without a byte clone or mutable-digest reconstruction. The
exact 69-centroid / 1,136-byte Random(17) fixture measured 5,512 → 1,464
B/candidate across three warmed 100,000-operation rounds (73.4% lower); this is
allocation evidence only. Immutable bytes, compact/capacity rewrites, and
legacy edge-case comparison coverage are retained. Validation: 556 tests,
genuine 3.2/3.3 interoperability, the 270-case oracle, scoped precommit and
independent review passed;
[details](https://github.com/apache/pinot/pull/19795#issuecomment-6069321460).
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/customobject/tdigest/SerializedTDigest.java:
##########
@@ -16,32 +16,34 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.pinot.segment.local.customobject;
+package org.apache.pinot.segment.local.customobject.tdigest;
-import com.tdunning.math.stats.TDigest;
-import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
import org.apache.pinot.spi.utils.BytesUtils;
import static com.google.common.base.Preconditions.checkArgument;
-/// Serialized and comparable version of TDigest. Compares TDigest for a
specific percentile value.
+/// Immutable serialized result and comparison key for a specific percentile
value.
+/// Uses Double.compare ordering: NaN quantiles sort last ascending and first
descending, ahead of positive infinity.
public class SerializedTDigest implements Comparable<SerializedTDigest> {
private final double _percentile;
- private final TDigest _tDigest;
+ private final byte[] _serializedBytes;
+ private final double _quantile;
public SerializedTDigest(TDigest tDigest, double percentile) {
- _tDigest = tDigest;
_percentile = percentile / 100.0;
+ // Finalize once before obtaining the sort key; serialization also owns
historical byte-retention policy.
+ _serializedBytes = tDigest.serialize();
Review Comment:
Fixed in 32f388a46e: RAW extraction finalizes a snapshot, preserving the
live reducer’s buffers and merge trajectory. The new regression verifies
repeated trims end byte-for-byte equal to an untouched reducer, including the
infinity wrapper. Your seeded fixture reproduced 0.00421875 P50 rank error
before the fix; the fixed result is 0.002390625, equal to untouched
aggregation. The working-quantile control remains 0.001703125 because it
flushes live state. Validation: 556 tests, genuine 3.2/3.3 interoperability,
the 270-case oracle, scoped precommit and independent review passed;
[details](https://github.com/apache/pinot/pull/19795#issuecomment-6069321460).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
