Skip to content

Commit 96534e4

Browse files
authored
Merge pull request #89 from koriym/claude/nostalgic-napier-6ec936
Fix silent debug-session failures and related reliability/perf issues
2 parents fecb9b0 + bf3b3d5 commit 96534e4

9 files changed

Lines changed: 797 additions & 216 deletions

File tree

bin/xprofile

Lines changed: 71 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -241,60 +241,85 @@ function analyzeCachegrindFile(string $profileFile, ?string $includeVendor = nul
241241
'mysql_query', 'pg_query', 'sqlite_query'
242242
];
243243

244-
// Simple file-based statistics (Cachegrind format is different from trace format)
245-
$content = file_get_contents($profileFile);
246-
$lines = explode("\n", $content);
247-
248-
$stats['total_lines'] = count($lines);
249-
250-
// Read the cachegrind `events:` header to locate the Time and Memory columns,
251-
// then take real measured totals from the `summary:` line. Units come from the
252-
// header (e.g. Time_(10ns), Memory_(bytes)); reading indices from the header
253-
// tolerates a non-default xdebug.cachegrind_events column order.
244+
// Stream the Cachegrind file line by line instead of loading it whole:
245+
// profiles can be hundreds of MB, and file_get_contents() + explode() held
246+
// 2-3x the file size in memory at once. The events:/summary: headers are
247+
// parsed inline the first time they are seen; the fn=/fl=/cost body below
248+
// is unchanged.
249+
//
250+
// Units for Time/Memory come from the events header (e.g. Time_(10ns),
251+
// Memory_(bytes)); reading column indices from the header tolerates a
252+
// non-default xdebug.cachegrind_events column order.
254253
$timeIndex = null;
255254
$memoryIndex = null;
256255
$timeToMs = null;
257-
if (preg_match('/^events:\s*(.+)$/m', $content, $eventsMatch)) {
258-
foreach (preg_split('/\s+/', trim($eventsMatch[1])) as $i => $col) {
259-
if (preg_match('/^Time_\((\d*)(ns|us|µs|ms|s)\)$/', $col, $unit)) {
260-
$timeIndex = $i;
261-
$multiplier = $unit[1] === '' ? 1 : (int) $unit[1];
262-
$nsPerUnit = ['ns' => 1, 'us' => 1000, 'µs' => 1000, 'ms' => 1000000, 's' => 1000000000][$unit[2]] ?? 1;
263-
$timeToMs = ($multiplier * $nsPerUnit) / 1000000.0;
264-
} elseif (str_starts_with($col, 'Memory_(')) {
265-
$memoryIndex = $i;
266-
}
267-
}
268-
}
269-
270256
$measuredTimeMs = null;
271257
$measuredMemoryMb = null;
272-
if (preg_match('/^summary:\s+(.+)$/m', $content, $summaryMatch)) {
273-
$summaryCols = preg_split('/\s+/', trim($summaryMatch[1]));
274-
if ($timeIndex !== null && $timeToMs !== null && isset($summaryCols[$timeIndex])) {
275-
$measuredTimeMs = round((int) $summaryCols[$timeIndex] * $timeToMs, 3);
276-
}
277-
if ($memoryIndex !== null && isset($summaryCols[$memoryIndex])) {
278-
$measuredMemoryMb = round((int) $summaryCols[$memoryIndex] / 1048576, 2);
279-
}
280-
}
281-
282-
// Count basic Cachegrind elements - accurate counting
283-
$stats['total_calls'] = substr_count($content, "\ncalls=");
284-
258+
$eventsParsed = false;
259+
$summaryParsed = false;
260+
$newlineCount = 0;
261+
285262
// Use sets to track unique functions
286263
$uniqueFunctions = [];
287264
$userFunctions = [];
288265
$internalFunctions = [];
289-
266+
290267
// Parse functions for classification and analysis
291268
$functionCosts = [];
292269
$currentFunction = null;
293270
$currentFile = null;
294271
$fileAliases = [];
295-
296-
foreach ($lines as $line) {
297-
$line = trim($line);
272+
273+
$handle = fopen($profileFile, 'r');
274+
if ($handle === false) {
275+
throw new \RuntimeException("Cannot open profile file: $profileFile");
276+
}
277+
278+
while (($rawLine = fgets($handle)) !== false) {
279+
// total_lines must stay byte-identical to the previous
280+
// count(explode("\n", $content)) == (number of "\n") + 1.
281+
if (str_ends_with($rawLine, "\n")) {
282+
$newlineCount++;
283+
}
284+
285+
$line = trim($rawLine);
286+
287+
// events: header — parse the first occurrence to locate the columns.
288+
if (! $eventsParsed && str_starts_with($line, 'events:')) {
289+
$eventsParsed = true;
290+
foreach (preg_split('/\s+/', trim(substr($line, strlen('events:')))) as $i => $col) {
291+
if (preg_match('/^Time_\((\d*)(ns|us|µs|ms|s)\)$/', $col, $unit)) {
292+
$timeIndex = $i;
293+
$multiplier = $unit[1] === '' ? 1 : (int) $unit[1];
294+
$nsPerUnit = ['ns' => 1, 'us' => 1000, 'µs' => 1000, 'ms' => 1000000, 's' => 1000000000][$unit[2]] ?? 1;
295+
$timeToMs = ($multiplier * $nsPerUnit) / 1000000.0;
296+
} elseif (str_starts_with($col, 'Memory_(')) {
297+
$memoryIndex = $i;
298+
}
299+
}
300+
301+
continue;
302+
}
303+
304+
// summary: line — real measured totals (appears once, near the end).
305+
if (! $summaryParsed && str_starts_with($line, 'summary:')) {
306+
$summaryParsed = true;
307+
$summaryCols = preg_split('/\s+/', trim(substr($line, strlen('summary:'))));
308+
if ($timeIndex !== null && $timeToMs !== null && isset($summaryCols[$timeIndex])) {
309+
$measuredTimeMs = round((int) $summaryCols[$timeIndex] * $timeToMs, 3);
310+
}
311+
if ($memoryIndex !== null && isset($summaryCols[$memoryIndex])) {
312+
$measuredMemoryMb = round((int) $summaryCols[$memoryIndex] / 1048576, 2);
313+
}
314+
315+
continue;
316+
}
317+
318+
// Count cost-block "calls=" markers (was substr_count($content, "\ncalls=")).
319+
if (str_starts_with($line, 'calls=')) {
320+
$stats['total_calls']++;
321+
continue;
322+
}
298323

299324
if (preg_match('/^fl=\((\d+)\)\s+(.+)$/', $line, $matches)) {
300325
$currentFile = $matches[2];
@@ -377,7 +402,12 @@ function analyzeCachegrindFile(string $profileFile, ?string $includeVendor = nul
377402
+ (int) ($costCols[$timeCol] ?? 0);
378403
}
379404
}
380-
405+
406+
fclose($handle);
407+
408+
// Preserve the previous count(explode("\n", $content)) semantics exactly.
409+
$stats['total_lines'] = $newlineCount + 1;
410+
381411
// Set final counts from unique arrays
382412
$stats['functions_count'] = count($uniqueFunctions);
383413
$stats['user_functions'] = count($userFunctions);

src/CompareRunner.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
use function preg_match;
2727
use function proc_close;
2828
use function proc_open;
29+
use function register_shutdown_function;
2930
use function sort;
3031
use function stream_get_contents;
3132
use function strlen;
@@ -205,6 +206,17 @@ private function createWorktree(string $ref): string
205206
$tempDir = sys_get_temp_dir() . '/xcompare-' . uniqid('', true);
206207
$this->worktreePath = $tempDir;
207208

209+
// run()'s try/finally already removes the worktree on a normal return or
210+
// a thrown exception. This shutdown handler additionally covers a fatal
211+
// error (the finally never runs then), so the worktree is not leaked.
212+
// cleanupWorktree() nulls worktreePath, so a normal run makes this a
213+
// no-op. NOTE: a SIGTERM/SIGKILL delivered while blocked in
214+
// executeXstep()'s proc_close() still cannot be cleaned here — PHP
215+
// shutdown functions do not run on signal-default termination.
216+
register_shutdown_function(function (): void {
217+
$this->cleanupWorktree();
218+
});
219+
208220
$output = [];
209221
$exitCode = 0;
210222
exec('git worktree add --detach ' . escapeshellarg($tempDir) . ' ' . escapeshellarg($ref) . ' 2>&1', $output, $exitCode);

0 commit comments

Comments
 (0)