mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-14 04:08:05 +00:00
perf(r): look up source() targets by map instead of indexOf per import
resolveRImportInternal ran indexOf over the whole normalised file list for every source() import (O(files) each). The resolver already receives getWorkspaceFileIndex's per-pass normToRaw map (first raw spelling per normalised path, built once), so use it for the exact-path lookups; the legacy array-only call shape keeps the indexOf fallback. Behaviour is identical (both pick the first file in list order). New unit tests run one corpus (relative, ../, absolute, backslash and duplicate spellings, missing, suffix fallback) through both paths and through the live adapter, and assert the indexed path performs no indexOf scan (fails when the change is reverted).
This commit is contained in:
parent
0d6c86e35d
commit
30caa37906
2 changed files with 133 additions and 6 deletions
|
|
@ -20,6 +20,11 @@ import { getWorkspaceFileIndex } from './workspace-file-index.js';
|
|||
/**
|
||||
* Low-level R import resolver (internal helper).
|
||||
* Resolves library/require/source paths to matching .R files.
|
||||
*
|
||||
* `normToRaw` (normalized path → first raw path with that spelling, built once per pass by
|
||||
* `getWorkspaceFileIndex`) answers the exact-path lookups of `source()` in O(1). Without it
|
||||
* they fall back to `indexOf` over `normalizedFileList`, which picks the same file (the first
|
||||
* in list order) but costs O(files) per `source()` import.
|
||||
*/
|
||||
export function resolveRImportInternal(
|
||||
filePath: string,
|
||||
|
|
@ -28,20 +33,25 @@ export function resolveRImportInternal(
|
|||
allFileList: readonly string[],
|
||||
rConfig: RPackageConfig | null,
|
||||
index?: SuffixIndex,
|
||||
normToRaw?: ReadonlyMap<string, string>,
|
||||
): string | string[] | null {
|
||||
const exactFile = (normalizedPath: string): string | undefined => {
|
||||
if (normToRaw !== undefined) return normToRaw.get(normalizedPath);
|
||||
const idx = normalizedFileList.indexOf(normalizedPath);
|
||||
return idx >= 0 ? allFileList[idx] : undefined;
|
||||
};
|
||||
const cleaned = rawImportPath.replace(/^["']|["']$/g, '');
|
||||
|
||||
// source() with file path
|
||||
if (cleaned.endsWith('.R') || cleaned.endsWith('.r')) {
|
||||
if (path.isAbsolute(cleaned)) {
|
||||
const normalized = cleaned.replace(/\\/g, '/');
|
||||
const idx = normalizedFileList.indexOf(normalized);
|
||||
return idx >= 0 ? allFileList[idx] : null;
|
||||
return exactFile(normalized) ?? null;
|
||||
}
|
||||
const dir = path.dirname(filePath);
|
||||
const resolved = path.join(dir, cleaned).replace(/\\/g, '/');
|
||||
const idx = normalizedFileList.indexOf(resolved);
|
||||
if (idx >= 0) return allFileList[idx];
|
||||
const direct = exactFile(resolved);
|
||||
if (direct !== undefined) return direct;
|
||||
|
||||
const pathParts = cleaned.split('/').filter(Boolean);
|
||||
return suffixResolve(pathParts, normalizedFileList, allFileList, index);
|
||||
|
|
@ -234,6 +244,6 @@ export function resolveRImportTarget(
|
|||
);
|
||||
}
|
||||
|
||||
const { normalized, all, index } = getWorkspaceFileIndex(allFilePaths);
|
||||
return resolveRImportInternal(fromFile, targetRaw, normalized, all, rConfig, index);
|
||||
const { normalized, all, index, normToRaw } = getWorkspaceFileIndex(allFilePaths);
|
||||
return resolveRImportInternal(fromFile, targetRaw, normalized, all, rConfig, index, normToRaw);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,117 @@
|
|||
/**
|
||||
* R `source("file.R")` exact-path lookup. The live resolver answers it from the per-pass
|
||||
* `normToRaw` map instead of `indexOf` over the whole file list for every `source()` call.
|
||||
* The map must pick exactly the file `indexOf` picked (first in list order), so these cases
|
||||
* compare both paths on the same corpus, including the duplicate-spelling and missing-file
|
||||
* cases where an index could plausibly diverge, and pin that the hot path no longer scans.
|
||||
*/
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
import {
|
||||
resolveRImportInternal,
|
||||
resolveRImportTarget,
|
||||
} from '../../../../src/core/ingestion/import-resolvers/r.js';
|
||||
import { getWorkspaceFileIndex } from '../../../../src/core/ingestion/import-resolvers/workspace-file-index.js';
|
||||
|
||||
// Raw spellings: the Windows-style duplicate of 'pkg/R/dup.R' comes FIRST, so first-wins must
|
||||
// return the backslash spelling for both lookups.
|
||||
const FILES = [
|
||||
'scripts\\win.R',
|
||||
'pkg\\R\\dup.R',
|
||||
'pkg/R/dup.R',
|
||||
'pkg/R/a.R',
|
||||
'pkg/R/b.R',
|
||||
'scripts/run.R',
|
||||
'scripts/helpers/util.R',
|
||||
'util.R',
|
||||
'vendor/other/util.R',
|
||||
'/abs/root.R',
|
||||
];
|
||||
|
||||
interface Case {
|
||||
readonly title: string;
|
||||
readonly from: string;
|
||||
readonly raw: string;
|
||||
}
|
||||
|
||||
const CASES: readonly Case[] = [
|
||||
{ title: 'relative, found beside the caller', from: 'pkg/R/a.R', raw: '"b.R"' },
|
||||
{ title: 'relative into a subdirectory', from: 'scripts/run.R', raw: '"helpers/util.R"' },
|
||||
{ title: 'relative with ../', from: 'scripts/helpers/x.R', raw: '"../run.R"' },
|
||||
{ title: 'duplicate spellings: first wins', from: 'pkg/R/a.R', raw: '"dup.R"' },
|
||||
{ title: 'backslash spelling of the file', from: 'scripts/x.R', raw: '"win.R"' },
|
||||
{ title: 'absolute, present', from: 'pkg/R/a.R', raw: '"/abs/root.R"' },
|
||||
{ title: 'absolute, absent', from: 'pkg/R/a.R', raw: '"/abs/missing.R"' },
|
||||
{ title: 'relative, absent: suffix fallback', from: 'pkg/R/a.R', raw: '"util.R"' },
|
||||
{ title: 'relative, absent everywhere', from: 'pkg/R/a.R', raw: '"nowhere.R"' },
|
||||
{ title: 'lowercase extension', from: 'pkg/R/a.R', raw: '"a.r"' },
|
||||
];
|
||||
|
||||
const run = (c: Case, withMap: boolean): ReturnType<typeof resolveRImportInternal> => {
|
||||
const { normalized, all, index, normToRaw } = getWorkspaceFileIndex(new Set(FILES));
|
||||
return resolveRImportInternal(
|
||||
c.from,
|
||||
c.raw,
|
||||
normalized,
|
||||
all,
|
||||
null,
|
||||
index,
|
||||
withMap ? normToRaw : undefined,
|
||||
);
|
||||
};
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe('resolveRImportInternal: source() exact-path lookup', () => {
|
||||
it.each(CASES)('$title: the index agrees with indexOf', (c) => {
|
||||
expect(run(c, true)).toEqual(run(c, false));
|
||||
});
|
||||
|
||||
it('positive controls: the corpus really exercises found, duplicate and missing files', () => {
|
||||
const byTitle = (title: string) => run(CASES.find((c) => c.title === title)!, true);
|
||||
expect(byTitle('relative, found beside the caller')).toBe('pkg/R/b.R');
|
||||
expect(byTitle('duplicate spellings: first wins')).toBe('pkg\\R\\dup.R');
|
||||
expect(byTitle('backslash spelling of the file')).toBe('scripts\\win.R');
|
||||
expect(byTitle('absolute, present')).toBe('/abs/root.R');
|
||||
expect(byTitle('absolute, absent')).toBeNull();
|
||||
expect(byTitle('relative, absent everywhere')).toBeNull();
|
||||
});
|
||||
|
||||
it('does not scan the file list for an exact hit when the index is supplied', () => {
|
||||
const { normalized, all, index, normToRaw } = getWorkspaceFileIndex(new Set(FILES));
|
||||
const indexOf = vi.spyOn(normalized, 'indexOf');
|
||||
expect(
|
||||
resolveRImportInternal('pkg/R/a.R', '"b.R"', normalized, all, null, index, normToRaw),
|
||||
).toBe('pkg/R/b.R');
|
||||
expect(
|
||||
resolveRImportInternal('pkg/R/a.R', '"/abs/root.R"', normalized, all, null, index, normToRaw),
|
||||
).toBe('/abs/root.R');
|
||||
expect(indexOf).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('still scans (and agrees) when called without the index', () => {
|
||||
const { normalized, all, index } = getWorkspaceFileIndex(new Set(FILES));
|
||||
const indexOf = vi.spyOn(normalized, 'indexOf');
|
||||
expect(resolveRImportInternal('pkg/R/a.R', '"b.R"', normalized, all, null, index)).toBe(
|
||||
'pkg/R/b.R',
|
||||
);
|
||||
expect(indexOf).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveRImportTarget: source() through the live adapter', () => {
|
||||
// `source()` carries no wildcard/named context, so the adapter takes the file-path branch.
|
||||
it.each(CASES)('$title: matches the indexOf result', (c) => {
|
||||
const viaAdapter = resolveRImportTarget(c.raw, c.from, new Set(FILES));
|
||||
expect(viaAdapter).toEqual(run(c, false));
|
||||
});
|
||||
|
||||
it('does not scan the file list on a repeat call for the same file set', () => {
|
||||
const files = new Set(FILES);
|
||||
resolveRImportTarget('"b.R"', 'pkg/R/a.R', files); // builds the per-set index
|
||||
const indexOf = vi.spyOn(Array.prototype, 'indexOf');
|
||||
expect(resolveRImportTarget('"b.R"', 'pkg/R/a.R', files)).toBe('pkg/R/b.R');
|
||||
expect(indexOf.mock.calls.filter(([needle]) => needle === 'pkg/R/b.R')).toEqual([]);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue