mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-31 06:52:49 +00:00
fix(r): look up backticked NAMESPACE exports from a set built once
Resolving an importFrom() name that the provider's NAMESPACE does not export walked every export entry before giving up, so a provider with 20,000 exports and a caller importing 20,000 names took about two seconds, and NAMESPACE text is the only bound. The walk only looked for entries that start or end with a backtick (a quoted "`%+%`" entry keeps them). Those are now collected in one pass per export set, and a lookup is a set membership test; the answers are unchanged.
This commit is contained in:
parent
055466b2ed
commit
23eb44dd13
2 changed files with 122 additions and 1 deletions
|
|
@ -165,6 +165,31 @@ const stripBackticks = (s: string): string => {
|
|||
return s.slice(start, end);
|
||||
};
|
||||
|
||||
/** The backticked entries of one export set, stripped, and the set size they were read at. */
|
||||
const backtickedExportSymbols = new WeakMap<
|
||||
ReadonlySet<string>,
|
||||
{ readonly size: number; readonly symbols: ReadonlySet<string> }
|
||||
>();
|
||||
|
||||
/**
|
||||
* The symbols of the entries of `exports` that start or end with a backtick (a quoted
|
||||
* `` "`%+%`" `` entry keeps its backticks), stripped. Built in one pass per export set, so a
|
||||
* lookup is a set membership test instead of a walk over every export; a set that has grown or
|
||||
* shrunk since is read again.
|
||||
*/
|
||||
function backtickedExports(exports: ReadonlySet<string>): ReadonlySet<string> {
|
||||
const cached = backtickedExportSymbols.get(exports);
|
||||
if (cached !== undefined && cached.size === exports.size) return cached.symbols;
|
||||
const symbols = new Set<string>();
|
||||
for (const entry of exports) {
|
||||
if (entry.charCodeAt(0) === 96 || entry.charCodeAt(entry.length - 1) === 96) {
|
||||
symbols.add(stripBackticks(entry));
|
||||
}
|
||||
}
|
||||
backtickedExportSymbols.set(exports, { size: exports.size, symbols });
|
||||
return symbols;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a NAMESPACE `importFrom(pkg, importedName)` to the `R/` file(s) of the
|
||||
* local package `pkg` that define `importedName` at top level, or `null`.
|
||||
|
|
@ -199,7 +224,7 @@ export function resolveRNamedImport(
|
|||
if (info !== undefined && info.hasNamespaceFile) {
|
||||
const exported =
|
||||
info.namedExports.has(symbol) ||
|
||||
[...info.namedExports].some((entry) => stripBackticks(entry) === symbol) ||
|
||||
backtickedExports(info.namedExports).has(symbol) ||
|
||||
rExportPatternMatches(info, symbol);
|
||||
if (!exported) return null;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,96 @@
|
|||
/**
|
||||
* Looking up an `importFrom()` name that a provider's NAMESPACE does not export must cost one set
|
||||
* lookup, not a pass over every export: a NAMESPACE can carry any number of `export()` entries and
|
||||
* a caller any number of `importFrom()` names, so a scan per lookup is quadratic in repository text.
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import type { ParsedFile } from 'gitnexus-shared';
|
||||
import { extractParsedFile } from '../../../../src/core/ingestion/scope-extractor-bridge.js';
|
||||
import { rProvider } from '../../../../src/core/ingestion/languages/r.js';
|
||||
import type {
|
||||
RNamespaceInfo,
|
||||
RPackageConfig,
|
||||
} from '../../../../src/core/ingestion/languages/r/package-config.js';
|
||||
import { resolveRNamedImport } from '../../../../src/core/ingestion/import-resolvers/r.js';
|
||||
|
||||
/** A set that counts every way of walking all of its entries. */
|
||||
class CountingSet extends Set<string> {
|
||||
walks = 0;
|
||||
override [Symbol.iterator](): SetIterator<string> {
|
||||
this.walks++;
|
||||
return super[Symbol.iterator]();
|
||||
}
|
||||
override values(): SetIterator<string> {
|
||||
this.walks++;
|
||||
return super.values();
|
||||
}
|
||||
override entries(): SetIterator<[string, string]> {
|
||||
this.walks++;
|
||||
return super.entries();
|
||||
}
|
||||
override forEach(...args: Parameters<Set<string>['forEach']>): void {
|
||||
this.walks++;
|
||||
super.forEach(...args);
|
||||
}
|
||||
}
|
||||
|
||||
function setup(exportCount: number): {
|
||||
cfg: RPackageConfig;
|
||||
exports: CountingSet;
|
||||
file: ParsedFile;
|
||||
} {
|
||||
const exports = new CountingSet();
|
||||
for (let i = 0; i < exportCount; i++) exports.add(`exported_${String(i)}`);
|
||||
const info: RNamespaceInfo = {
|
||||
hasNamespaceFile: true,
|
||||
namedExports: exports,
|
||||
exportPatterns: [],
|
||||
importFrom: [],
|
||||
};
|
||||
const cfg: RPackageConfig = {
|
||||
packages: new Map([['prov', 'prov']]),
|
||||
namespaceInfoByPackageDir: new Map([['prov', info]]),
|
||||
};
|
||||
const file = extractParsedFile(
|
||||
rProvider,
|
||||
'exported_5 <- function() 1\nprivate_fn <- function() 2',
|
||||
'prov/R/a.R',
|
||||
);
|
||||
if (file === undefined) throw new Error('no ParsedFile');
|
||||
return { cfg, exports, file };
|
||||
}
|
||||
|
||||
describe('resolveRNamedImport: the cost of a name the NAMESPACE does not export', () => {
|
||||
it('walks the export set once for any number of lookups of names that are not exported', () => {
|
||||
const { cfg, exports, file } = setup(1_000);
|
||||
for (let i = 0; i < 100; i++) {
|
||||
expect(resolveRNamedImport(cfg, 'prov', `private_fn_${String(i)}`, [file])).toBeNull();
|
||||
}
|
||||
expect(resolveRNamedImport(cfg, 'prov', 'private_fn', [file])).toBeNull();
|
||||
// One pass builds the index of backticked entries; a scan per lookup would be 101 walks.
|
||||
expect(exports.walks).toBe(1);
|
||||
});
|
||||
|
||||
it('still matches a backticked entry, and reads the set again after it changed', () => {
|
||||
const { cfg, exports, file } = setup(10);
|
||||
expect(resolveRNamedImport(cfg, 'prov', 'private_fn', [file])).toBeNull();
|
||||
exports.add('`private_fn`');
|
||||
expect(resolveRNamedImport(cfg, 'prov', 'private_fn', [file])).toEqual(['prov/R/a.R']);
|
||||
});
|
||||
|
||||
it('does not walk the export set for an exported name', () => {
|
||||
const { cfg, exports, file } = setup(1_000);
|
||||
expect(resolveRNamedImport(cfg, 'prov', 'exported_5', [file])).toEqual(['prov/R/a.R']);
|
||||
expect(exports.walks).toBe(0);
|
||||
});
|
||||
|
||||
it('answers 20,000 lookups against 20,000 exports well inside a generous bound', () => {
|
||||
const { cfg, file } = setup(20_000);
|
||||
const start = performance.now();
|
||||
for (let i = 0; i < 20_000; i++) {
|
||||
resolveRNamedImport(cfg, 'prov', `private_fn_${String(i)}`, [file]);
|
||||
}
|
||||
// One set lookup each: a few milliseconds. The scan this replaced took about two seconds.
|
||||
expect(performance.now() - start).toBeLessThan(500);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue