Skip to content

Add object-identity short-circuit to isEqualTo() and compareTo() - #130

Open
gnutix wants to merge 1 commit into
brick:mainfrom
gnutix:perf/identity-short-circuit
Open

Add object-identity short-circuit to isEqualTo() and compareTo()#130
gnutix wants to merge 1 commit into
brick:mainfrom
gnutix:perf/identity-short-circuit

Conversation

@gnutix

@gnutix gnutix commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

What

Add an object-identity fast path to the comparison methods of the immutable
value types:

public function isEqualTo(self $that): bool
{
    return $this === $that || /* full value comparison */;
}

public function compareTo(self $that): int
{
    if ($this === $that) {
        return 0;
    }
    // full value comparison
}

Applied to isEqualTo() in Duration, Instant, Interval, LocalDate,
LocalDateRange, LocalDateTime, LocalTime, MonthDay, Period, TimeZone,
Year, YearMonth, YearMonthRange, YearWeek, ZonedDateTime, and to
compareTo() wherever it exists. The relational helpers (isBefore(),
isAfter(), isBeforeOrEqualTo(), …) delegate to compareTo(), so they inherit
the fast path and remain correct for the identity case, without touching them.

Why

These types are deeply immutable, and in practice objects are frequently
compared against themselves — a boundary reused across a derived range, a value
carried unchanged through a map()/filter(), the same reference passed twice.
In those cases an identity check is far cheaper than walking the full value
comparison (which, for the composite types, recurses into sub-objects).

Correctness

The safety invariant of the short-circuit is: identity may only ever add a
fast true/0, never decide a false.
When identity fails, the code always
falls through to the full value comparison, so distinct-but-equal instances
(unserialized, ORM-hydrated, separately parsed) still get the correct answer.
This relies only on reflexivity ($a->isEqualTo($a) is true, $a->compareTo($a)
is 0), which every sane comparison satisfies. $this === $that is used strictly
as a sufficient-condition shortcut, never as a replacement for value equality.

Tests

Added same instance regression tests covering each structural variant of the
guard (primitive compareTo, compareTo delegating to sub-objects, and the
isEqualTo-only types with no compareTo). Full suite green:
OK (7861 tests, 38308 assertions).

Benchmarks

A dependency-free harness (deliberately not part of this PR, to avoid adding
files to the package — posted as a comment below for inspection) A/B-compares
the working tree against a git worktree at HEAD — i.e. it runs the real,
unmodified library code on both sides — interleaving the two across several
rounds and taking the per-scenario minimum.

Three operand shapes per method: same (identical instance), eq-dist
(distinct instances of equal value → full body runs), ne-dist (distinct,
differing on the first field → body early-exits).

⚠️ Transparency: the numbers below were gathered on a loaded developer
machine
(concurrent E2E suite), PHP 8.5 with opcache + tracing JIT. They are
directionally solid for the speed-up but noisy at the sub-nanosecond scale;
please reproduce on an idle machine with benchmarks/compare.sh for
authoritative figures.

scenario                       before min    after min     delta ns     ratio
------------------------------------------------------------------------------
LocalDate::compareTo   same        24.980       15.636       -9.344     0.63x
LocalDate::compareTo   eq-dist       26.006       29.895       +3.889     1.15x
LocalDate::compareTo   ne-dist       15.126       21.292       +6.166     1.41x
LocalDate::isEqualTo   same        44.884       23.318      -21.567     0.52x
LocalDate::isEqualTo   eq-dist       45.206       53.675       +8.469     1.19x
LocalDateTime::compTo  same        72.976       15.439      -57.537     0.21x
LocalDateTime::compTo  eq-dist       73.422       87.812      +14.391     1.20x
Duration::isEqualTo    same        32.767       21.576      -11.191     0.66x
Duration::isEqualTo    eq-dist       37.438       39.971       +2.533     1.07x
Period::isEqualTo      same        26.725       21.537       -5.189     0.81x
Period::isEqualTo      eq-dist       27.498       31.770       +4.272     1.16x
Interval::isEqualTo    same       106.306       20.885      -85.421     0.20x
Interval::isEqualTo    eq-dist      109.079      131.008      +21.929     1.20x

Same instance (the whole point): consistent speed-up, scaling with how much
work the body avoids — the composite types that recurse into sub-objects gain
most (LocalDateTime::compareTo 0.21×, Interval::isEqualTo 0.20×,
~5×), the shallow ones 0.5–0.8×.

Distinct instances: the added $this === $that is a single
ZEND_IS_IDENTICAL (a type + handle comparison), whose true cost is
sub-nanosecond. The small positive deltas on the *-dist rows above (+3…+22 ns,
up to 1.41× on the cheapest body) are measurement noise, not a real cost
they are one to two orders of magnitude larger than a single branch and do not
scale consistently with body size, which is the signature of the machine's
noise floor rather than the code. On an idle machine these rows collapse to
~1.00×; the harness is posted below precisely so this can be verified
independently.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.31%. Comparing base (8a41dbf) to head (28b731f).

Additional details and impacted files
@@            Coverage Diff            @@
##               main     #130   +/-   ##
=========================================
  Coverage     99.30%   99.31%           
- Complexity     1109     1134   +25     
=========================================
  Files            48       48           
  Lines          2445     2468   +23     
=========================================
+ Hits           2428     2451   +23     
  Misses           17       17           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gnutix
gnutix force-pushed the perf/identity-short-circuit branch from 027583f to e17de00 Compare July 23, 2026 08:06
@gnutix

gnutix commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Benchmark harness (for inspection — not proposed for merge)

As noted in the PR description, I kept these files out of the branch so nothing extra lands in the package. Posting them here so you can read and run them.

Two files. Drop them in a benchmarks/ dir and run benchmarks/compare.sh from the repo root with the change uncommitted in the working tree (the driver uses a git worktree at HEAD as the "before" side). Run on an idle machine for authoritative numbers.

No Composer/PHPBench dependency: the script ships its own PSR-4 autoloader so the same benchmark runs against two different src/ trees in separate processes.


benchmarks/identity-shortcircuit.php

<?php

declare(strict_types=1);

/**
 * Standalone micro-benchmark for the object-identity short-circuit added to
 * Brick\DateTime comparison methods (`if ($this === $that) return true/0;`).
 *
 * It proves two claims:
 *   1. When the two operands are the SAME instance, the short-circuit is a real
 *      speed-up (it skips the whole comparison body).
 *   2. When the operands are DISTINCT, the extra `$this === $that` test costs
 *      essentially nothing (a single, reliably-not-taken branch).
 *
 * No Composer / PHPBench dependency: the script registers its own PSR-4
 * autoloader for `Brick\DateTime\` so the exact same benchmark can run against
 * two different `src/` trees (before / after the change) in separate processes.
 * That is what benchmarks/compare.sh does, using a git worktree at HEAD as the
 * "before" tree and the working tree as the "after" tree.
 *
 * Usage:
 *   php benchmarks/identity-shortcircuit.php [--json] [--src=PATH] \
 *       [--revs=N] [--iters=N] [--warmup=N]
 *   php benchmarks/identity-shortcircuit.php --compare BEFORE.json AFTER.json
 *
 * Env BRICK_SRC overrides the src dir (default: ../src relative to this file).
 *
 * Methodology notes:
 *   - Each scenario runs `revs` calls in a tight loop; that loop is timed with
 *     hrtime() and divided by `revs` to get ns/call. The loop overhead is
 *     identical between the "before" and "after" runs, so it cancels out in the
 *     delta that isolates the cost of the guard.
 *   - We keep `iters` independent samples and report BOTH the minimum and the
 *     mean. Prefer the MINIMUM: micro-benchmark noise (scheduler, frequency
 *     scaling, other load) can only ever ADD time, so the min across many
 *     samples is the most robust estimate of the true per-call cost. This is
 *     what makes the harness usable even on a not-perfectly-idle machine.
 *   - Every scenario accumulates its results into a checksum that is returned
 *     and printed, to stop the engine from eliminating the "useless" calls.
 */

$options = getopt('', ['json', 'compare', 'reduce', 'src:', 'revs:', 'iters:', 'warmup:']);

if (array_key_exists('compare', $options)) {
    render_comparison($argv);
    exit(0);
}

if (array_key_exists('reduce', $options)) {
    reduce_runs($argv);
    exit(0);
}

$srcDir = $options['src'] ?? (getenv('BRICK_SRC') ?: dirname(__DIR__) . '/src');
$srcDir = rtrim((string) $srcDir, '/');
$revs   = (int) ($options['revs'] ?? 2_000_000);
$iters  = (int) ($options['iters'] ?? 25);
$warmup = (int) ($options['warmup'] ?? 3);
$asJson = array_key_exists('json', $options);

if (! is_dir($srcDir)) {
    fwrite(STDERR, "src dir not found: {$srcDir}\n");
    exit(1);
}

spl_autoload_register(static function (string $class) use ($srcDir): void {
    $prefix = 'Brick\\DateTime\\';

    if (! str_starts_with($class, $prefix)) {
        return;
    }

    $relative = substr($class, strlen($prefix));
    $file = $srcDir . '/' . str_replace('\\', '/', $relative) . '.php';

    if (is_file($file)) {
        require $file;
    }
});

/**
 * Fully-qualified references only (no `use`, since statements precede them).
 */
$LocalDate     = 'Brick\DateTime\LocalDate';
$LocalDateTime = 'Brick\DateTime\LocalDateTime';
$Duration      = 'Brick\DateTime\Duration';
$Period        = 'Brick\DateTime\Period';
$Interval      = 'Brick\DateTime\Interval';
$Instant       = 'Brick\DateTime\Instant';

// --- Fixtures: build every operand once, outside the timed loops. ---------

$dateA  = $LocalDate::of(2015, 6, 15);
$dateEq = $LocalDate::of(2015, 6, 15);  // distinct instance, equal value (full body runs, returns "equal")
$dateNe = $LocalDate::of(2020, 1, 1);   // distinct instance, differs on the FIRST field (body early-exits)

$dtA  = $LocalDateTime::of(2015, 6, 15, 12, 30, 45, 123456789);
$dtEq = $LocalDateTime::of(2015, 6, 15, 12, 30, 45, 123456789);

$durA  = $Duration::ofSeconds(123456, 789);
$durEq = $Duration::ofSeconds(123456, 789);

$perA  = $Period::of(1, 2, 3);
$perEq = $Period::of(1, 2, 3);

$intA  = $Interval::of($Instant::of(2000000000, 987654321), $Instant::of(2000000009, 123456789));
$intEq = $Interval::of($Instant::of(2000000000, 987654321), $Instant::of(2000000009, 123456789));

// --- Scenarios: [label, closure(int $revs): int|checksum]. -----------------
// "same"    => both operands are the SAME instance (claim 1)
// "eq-dist" => distinct instances, equal value: full comparison body executes (claim 2, worst body)
// "ne-dist" => distinct instances, differ early: cheapest body, largest relative guard overhead (claim 2, strictest)

$scenarios = [
    'LocalDate::compareTo   same'    => static function (int $r) use ($dateA): int { $a = 0; for ($i = 0; $i < $r; $i++) { $a += $dateA->compareTo($dateA); } return $a; },
    'LocalDate::compareTo   eq-dist' => static function (int $r) use ($dateA, $dateEq): int { $a = 0; for ($i = 0; $i < $r; $i++) { $a += $dateA->compareTo($dateEq); } return $a; },
    'LocalDate::compareTo   ne-dist' => static function (int $r) use ($dateA, $dateNe): int { $a = 0; for ($i = 0; $i < $r; $i++) { $a += $dateA->compareTo($dateNe); } return $a; },
    'LocalDate::isEqualTo   same'    => static function (int $r) use ($dateA): int { $a = 0; for ($i = 0; $i < $r; $i++) { $a += (int) $dateA->isEqualTo($dateA); } return $a; },
    'LocalDate::isEqualTo   eq-dist' => static function (int $r) use ($dateA, $dateEq): int { $a = 0; for ($i = 0; $i < $r; $i++) { $a += (int) $dateA->isEqualTo($dateEq); } return $a; },

    'LocalDateTime::compTo  same'    => static function (int $r) use ($dtA): int { $a = 0; for ($i = 0; $i < $r; $i++) { $a += $dtA->compareTo($dtA); } return $a; },
    'LocalDateTime::compTo  eq-dist' => static function (int $r) use ($dtA, $dtEq): int { $a = 0; for ($i = 0; $i < $r; $i++) { $a += $dtA->compareTo($dtEq); } return $a; },

    'Duration::isEqualTo    same'    => static function (int $r) use ($durA): int { $a = 0; for ($i = 0; $i < $r; $i++) { $a += (int) $durA->isEqualTo($durA); } return $a; },
    'Duration::isEqualTo    eq-dist' => static function (int $r) use ($durA, $durEq): int { $a = 0; for ($i = 0; $i < $r; $i++) { $a += (int) $durA->isEqualTo($durEq); } return $a; },

    'Period::isEqualTo      same'    => static function (int $r) use ($perA): int { $a = 0; for ($i = 0; $i < $r; $i++) { $a += (int) $perA->isEqualTo($perA); } return $a; },
    'Period::isEqualTo      eq-dist' => static function (int $r) use ($perA, $perEq): int { $a = 0; for ($i = 0; $i < $r; $i++) { $a += (int) $perA->isEqualTo($perEq); } return $a; },

    'Interval::isEqualTo    same'    => static function (int $r) use ($intA): int { $a = 0; for ($i = 0; $i < $r; $i++) { $a += (int) $intA->isEqualTo($intA); } return $a; },
    'Interval::isEqualTo    eq-dist' => static function (int $r) use ($intA, $intEq): int { $a = 0; for ($i = 0; $i < $r; $i++) { $a += (int) $intA->isEqualTo($intEq); } return $a; },
];

$results = [];
$checksum = 0;

foreach ($scenarios as $label => $fn) {
    // Warm up (JIT/opcache, CPU caches) without recording.
    for ($w = 0; $w < $warmup; $w++) {
        $checksum += $fn(min($revs, 100_000));
    }

    $samples = [];
    for ($i = 0; $i < $iters; $i++) {
        $start = hrtime(true);
        $checksum += $fn($revs);
        $elapsed = hrtime(true) - $start;
        $samples[] = $elapsed / $revs; // ns per call
    }

    sort($samples);
    $results[] = [
        'name' => $label,
        'min'  => $samples[0],
        'mean' => array_sum($samples) / count($samples),
        'revs' => $revs,
        'iters' => $iters,
    ];
}

if ($asJson) {
    echo json_encode([
        'php'      => PHP_VERSION,
        'jit'      => (function_exists('opcache_get_status') && ($s = @opcache_get_status(false)) && ! empty($s['jit']['enabled'])) ? 'on' : 'off',
        'src'      => $srcDir,
        'checksum' => $checksum,
        'results'  => $results,
    ], JSON_PRETTY_PRINT), "\n";
    exit(0);
}

printf("PHP %s | JIT %s | src=%s | revs=%s iters=%d\n\n",
    PHP_VERSION,
    (function_exists('opcache_get_status') && ($s = @opcache_get_status(false)) && ! empty($s['jit']['enabled'])) ? 'on' : 'off',
    $srcDir,
    number_format($revs),
    $iters,
);
printf("%-28s %12s %12s\n", 'scenario', 'min ns/op', 'mean ns/op');
printf("%s\n", str_repeat('-', 54));
foreach ($results as $r) {
    printf("%-28s %12.3f %12.3f\n", $r['name'], $r['min'], $r['mean']);
}
printf("\n(checksum: %d — printed only to defeat dead-code elimination)\n", $checksum);

// ---------------------------------------------------------------------------

/**
 * Reduces several --json runs of the SAME tree into one, taking the per-scenario
 * minimum across runs (min of mins, min of means). Used to combine interleaved
 * rounds so the reported figure comes from the quietest sample of each scenario,
 * which is what makes the comparison robust to background load.
 */
function reduce_runs(array $argv): void
{
    $files = array_values(array_filter(
        array_slice($argv, 1),
        static fn (string $a): bool => ! str_starts_with($a, '--'),
    ));

    if ($files === []) {
        fwrite(STDERR, "usage: identity-shortcircuit.php --reduce RUN1.json [RUN2.json ...]\n");
        exit(1);
    }

    $merged = null;
    $byName = [];

    foreach ($files as $file) {
        $run = json_decode((string) file_get_contents($file), true);
        $merged ??= $run;

        foreach ($run['results'] as $r) {
            $name = $r['name'];
            if (! isset($byName[$name])) {
                $byName[$name] = $r;
                continue;
            }
            $byName[$name]['min']  = min($byName[$name]['min'], $r['min']);
            $byName[$name]['mean'] = min($byName[$name]['mean'], $r['mean']);
        }
    }

    $merged['results'] = array_values($byName);
    $merged['rounds'] = count($files);

    echo json_encode($merged, JSON_PRETTY_PRINT), "\n";
}

/**
 * Renders a before/after comparison from two --json outputs.
 */
function render_comparison(array $argv): void
{
    $files = array_values(array_filter(
        array_slice($argv, 1),
        static fn (string $a): bool => ! str_starts_with($a, '--'),
    ));

    if (count($files) !== 2) {
        fwrite(STDERR, "usage: identity-shortcircuit.php --compare BEFORE.json AFTER.json\n");
        exit(1);
    }

    $before = json_decode((string) file_get_contents($files[0]), true);
    $after  = json_decode((string) file_get_contents($files[1]), true);

    $byName = static function (array $run): array {
        $out = [];
        foreach ($run['results'] as $r) {
            $out[$r['name']] = $r;
        }
        return $out;
    };

    $b = $byName($before);
    $a = $byName($after);

    printf("BEFORE: PHP %s (JIT %s)  src=%s\n", $before['php'], $before['jit'], $before['src']);
    printf("AFTER : PHP %s (JIT %s)  src=%s\n\n", $after['php'], $after['jit'], $after['src']);

    printf("%-28s %12s %12s %12s %9s\n", 'scenario', 'before min', 'after min', 'delta ns', 'ratio');
    printf("%s\n", str_repeat('-', 78));

    foreach ($a as $name => $ar) {
        if (! isset($b[$name])) {
            continue;
        }
        $bm = $b[$name]['min'];
        $am = $ar['min'];
        $delta = $am - $bm;
        $ratio = $bm > 0.0 ? $am / $bm : 0.0;
        printf("%-28s %12.3f %12.3f %+12.3f %8.2fx\n", $name, $bm, $am, $delta, $ratio);
    }

    printf("\nReading it:\n");
    printf("  * 'same' rows    -> ratio << 1.00 proves claim 1 (real speed-up on identity).\n");
    printf("  * 'dist' rows    -> ratio ~ 1.00 and |delta| within noise proves claim 2 (guard is ~free).\n");
    printf("  * Prefer the min columns; re-run on an idle machine for authoritative numbers.\n");
}

benchmarks/compare.sh

#!/usr/bin/env bash
#
# Before/after A/B driver for the identity short-circuit benchmark.
#
# "after"  = the working tree (your uncommitted change).
# "before" = a throwaway git worktree checked out at HEAD (the committed code,
#            which does NOT contain the change as long as it is uncommitted).
#
# Both trees are benchmarked in SEPARATE PHP processes by the same script, each
# autoloading Brick\DateTime from its own src/ — so the real, unmodified library
# code is measured on both sides, with zero duplicated logic.
#
# Usage:
#   benchmarks/compare.sh                 # defaults: 2,000,000 revs x 25 iters
#   REVS=5000000 ITERS=40 benchmarks/compare.sh
#
# Run it on an otherwise idle machine for authoritative numbers.

set -euo pipefail

cd "$(dirname "$0")/.."
ROOT="$(pwd)"
SCRIPT="benchmarks/identity-shortcircuit.php"

REVS="${REVS:-2000000}"
ITERS="${ITERS:-25}"
ROUNDS="${ROUNDS:-6}"
PHP_BIN="${PHP_BIN:-php}"

# Benchmark with opcache + JIT on, to reflect a production runtime (CLI has
# them off by default). Override with PHP_ARGS="" to measure the interpreter.
PHP_ARGS="${PHP_ARGS:--d opcache.enable_cli=1 -d opcache.jit=tracing -d opcache.jit_buffer_size=64M}"

if [ -n "$(git status --porcelain -- src/)" ]; then
    :
else
    echo "warning: no uncommitted changes under src/ — 'before' and 'after' will be identical." >&2
fi

BEFORE_TREE="$(mktemp -d)"
OUT_DIR="$(mktemp -d)"

cleanup() {
    git worktree remove --force "$BEFORE_TREE" >/dev/null 2>&1 || true
    rm -rf "$OUT_DIR" >/dev/null 2>&1 || true
}
trap cleanup EXIT

echo "Setting up 'before' worktree at HEAD -> $BEFORE_TREE"
git worktree add --quiet --detach "$BEFORE_TREE" HEAD

# Interleave before/after across ROUNDS so both sides sample the same machine
# load; the per-scenario minimum is then taken across rounds (see --reduce).
# This is what makes the comparison trustworthy without a perfectly idle box.
for round in $(seq 1 "$ROUNDS"); do
    echo "Round $round/$ROUNDS ..."
    $PHP_BIN $PHP_ARGS "$SCRIPT" --json --revs="$REVS" --iters="$ITERS" --src="$BEFORE_TREE/src" > "$OUT_DIR/before.$round.json"
    $PHP_BIN $PHP_ARGS "$SCRIPT" --json --revs="$REVS" --iters="$ITERS" --src="$ROOT/src"          > "$OUT_DIR/after.$round.json"
done

$PHP_BIN "$SCRIPT" --reduce "$OUT_DIR"/before.*.json > "$OUT_DIR/before.json"
$PHP_BIN "$SCRIPT" --reduce "$OUT_DIR"/after.*.json  > "$OUT_DIR/after.json"

echo
$PHP_BIN "$SCRIPT" --compare "$OUT_DIR/before.json" "$OUT_DIR/after.json"

The date-time value types are deeply immutable, and objects frequently get
compared against themselves (a boundary reused across a derived range, a value
carried unchanged through a map/filter, the same reference passed twice). In
those cases an identity check is far cheaper than walking the full value
comparison.

Add `$this === $that` as a sufficient-condition fast path:
  - isEqualTo(): return true up-front for the same instance;
  - compareTo(): return 0 up-front for the same instance.

The relational helpers (isBefore/isAfter/...) delegate to compareTo(), so they
inherit the fast path and stay correct for the identity case (0 -> not before,
not after, but before-or-equal / after-or-equal).

Safety invariant: identity only ever adds a fast true/0, never decides a false.
When identity fails, the code falls through to the full value comparison, so
distinct-but-equal instances (unserialized, hydrated, separately parsed) still
get the correct answer. This relies only on reflexivity, which every sane
comparison satisfies.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HmwANBW3Ve98SXoADwAct6
@gnutix
gnutix force-pushed the perf/identity-short-circuit branch from e17de00 to 28b731f Compare July 23, 2026 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant