Answer the review: walk the brightness order in memory order, and stop at the fifteenth label
The adversarial review confirmed a regression in this PR. Near the Sun, the label pass became three to four times slower than the scan and sort it replaced. Within about 11 pc of the Sun, and in any plan view zoomed tighter than that, the label radius clamps to 4 pc. That sphere holds a few dozen faint dwarfs deep in the brightness order, so the walk rarely finds fifteen stars to name and reads nearly the whole catalogue. Reading the star objects in brightness order jumps all over memory, so a full walk took 19-25 ms against the old 5-6 ms. The review also found that spreadLabels checked the label count at the top of its loop. After placing the fifteenth label it asked for a sixteenth candidate, which near the Sun can lie at the far end of the order. brightnessIndex now lays each star's position and id out beside the brightness order, in that order. The walk tests stars from those arrays in sequence and reads a star object only when it yields one. spreadLabels breaks straight after placing the fifteenth label. Measured on the real catalogue with the label logic reduced to what decides placement, camera at the given distance from the Sun (old sort / this PR as first pushed / now): 2 pc 4.9 / 24.7 / 2.5 ms 5 pc 5.7 / 23.0 / 3.1 ms 10 pc 6.3 / 18.2 / 0.95 ms 307 pc 22 / 0.01 / 0.00 ms (the opening view) The labels are identical in every case. Now faster than the old sort at every distance. A new scene test counts the candidates spreadLabels takes: exactly fifteen for fifteen labels. Negative controls, each caught: positions one axis off, ids in catalogue order, the selected star dropped, the radius edge excluded, and the count checked before taking a candidate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016jxMkwA2rbicdGxHosecYi
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import { brightestWithin, brightnessOrder } from './brightest';
|
||||
import { brightestWithin, brightnessIndex, brightnessOrder } from './brightest';
|
||||
|
||||
interface TestStar {
|
||||
id: number;
|
||||
@@ -46,11 +46,11 @@ describe('brightestWithin', () => {
|
||||
|
||||
it('yields exactly what filtering and then sorting the catalogue did, in the same order', () => {
|
||||
const stars = cloud(3000);
|
||||
const order = brightnessOrder(stars);
|
||||
const index = brightnessIndex(stars);
|
||||
const centre = { x: 12, y: -30, z: 5 };
|
||||
|
||||
for (const [radius, alwaysId] of [[40, null], [15, 7 * 2999], [0, 7 * 11], [500, null]] as const) {
|
||||
const lazy = Array.from(brightestWithin(stars, order, centre, radius, alwaysId), (star) => star.id);
|
||||
const lazy = Array.from(brightestWithin(stars, index, centre, radius, alwaysId), (star) => star.id);
|
||||
expect(lazy).toEqual(filterThenSort(stars, centre, radius, alwaysId));
|
||||
}
|
||||
});
|
||||
@@ -61,7 +61,7 @@ describe('brightestWithin', () => {
|
||||
{ id: 2, x: 3, y: 4.001, z: 0, magnitude: 0 }
|
||||
];
|
||||
|
||||
expect(Array.from(brightestWithin(stars, brightnessOrder(stars), { x: 0, y: 0, z: 0 }, 5, null), (star) => star.id)).toEqual([1]);
|
||||
expect(Array.from(brightestWithin(stars, brightnessIndex(stars), { x: 0, y: 0, z: 0 }, 5, null), (star) => star.id)).toEqual([1]);
|
||||
});
|
||||
|
||||
it('reads no further than the caller takes', () => {
|
||||
@@ -77,7 +77,7 @@ describe('brightestWithin', () => {
|
||||
});
|
||||
|
||||
const taken: number[] = [];
|
||||
for (const star of brightestWithin(counted, brightnessOrder(stars), { x: 0, y: 0, z: 0 }, 1000, null)) {
|
||||
for (const star of brightestWithin(counted, brightnessIndex(stars), { x: 0, y: 0, z: 0 }, 1000, null)) {
|
||||
taken.push(star.id);
|
||||
if (taken.length === 15) {
|
||||
break;
|
||||
|
||||
@@ -27,6 +27,38 @@ export function brightnessOrder(stars: readonly BrightnessRanked[]): Uint32Array
|
||||
return Uint32Array.from(stars.keys()).sort((a, b) => stars[a].magnitude - stars[b].magnitude);
|
||||
}
|
||||
|
||||
/**
|
||||
* The brightness order, with each star's position and id laid out beside it in that order.
|
||||
*
|
||||
* A walk has to test every star it passes, and near the Sun it passes nearly all of them: a 4 pc
|
||||
* label radius holds a few dozen stars, faint dwarfs deep in the order, so the walk rarely finds
|
||||
* fifteen to name before the end. Reading the stars themselves in brightness order jumps all over
|
||||
* the catalogue, and a full walk took 19-23 ms — slower than the scan and sort it replaced. Read
|
||||
* from these arrays, laid out in the order they are walked, the same walk touches memory in
|
||||
* sequence and reads a star only when it yields one.
|
||||
*/
|
||||
export interface BrightnessIndex {
|
||||
/** Indices into the catalogue, brightest first. */
|
||||
readonly order: Uint32Array;
|
||||
/** Positions in the same order, three to a star, at full precision so a star on a radius stays on it. */
|
||||
readonly positions: Float64Array;
|
||||
readonly ids: Float64Array;
|
||||
}
|
||||
|
||||
export function brightnessIndex<T extends BrightnessRanked & Positioned & { readonly id: number }>(stars: readonly T[]): BrightnessIndex {
|
||||
const order = brightnessOrder(stars);
|
||||
const positions = new Float64Array(order.length * 3);
|
||||
const ids = new Float64Array(order.length);
|
||||
order.forEach((index, at) => {
|
||||
const star = stars[index];
|
||||
positions[at * 3] = star.x;
|
||||
positions[at * 3 + 1] = star.y;
|
||||
positions[at * 3 + 2] = star.z;
|
||||
ids[at] = star.id;
|
||||
});
|
||||
return { order, positions, ids };
|
||||
}
|
||||
|
||||
/**
|
||||
* The stars within `radiusPc` of `centre`, brightest first, plus the one star `alwaysId` names
|
||||
* wherever it is — handed over lazily, so a caller that stops after the first few pays for no
|
||||
@@ -34,19 +66,19 @@ export function brightnessOrder(stars: readonly BrightnessRanked[]): Uint32Array
|
||||
*/
|
||||
export function* brightestWithin<T extends BrightnessRanked & Positioned & { readonly id: number }>(
|
||||
stars: readonly T[],
|
||||
order: Uint32Array,
|
||||
index: BrightnessIndex,
|
||||
centre: Positioned,
|
||||
radiusPc: number,
|
||||
alwaysId: number | null
|
||||
): Generator<T> {
|
||||
const { order, positions, ids } = index;
|
||||
const radiusSq = radiusPc * radiusPc;
|
||||
for (const index of order) {
|
||||
const star = stars[index];
|
||||
const dx = star.x - centre.x;
|
||||
const dy = star.y - centre.y;
|
||||
const dz = star.z - centre.z;
|
||||
if (dx * dx + dy * dy + dz * dz <= radiusSq || star.id === alwaysId) {
|
||||
yield star;
|
||||
for (let at = 0; at < order.length; at++) {
|
||||
const dx = positions[at * 3] - centre.x;
|
||||
const dy = positions[at * 3 + 1] - centre.y;
|
||||
const dz = positions[at * 3 + 2] - centre.z;
|
||||
if (dx * dx + dy * dy + dz * dz <= radiusSq || ids[at] === alwaysId) {
|
||||
yield stars[order[at]];
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user