Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: simple Grapher & CoreTable perf improvements #3910

Merged
merged 5 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions packages/@ourworldindata/core-table/src/CoreTableColumns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import {
imemo,
ToleranceStrategy,
IndicatorTitleWithFragments,
max,
min,
} from "@ourworldindata/utils"
import { CoreTable } from "./CoreTable.js"
import {
Expand Down Expand Up @@ -499,12 +501,12 @@ export abstract class AbstractCoreColumn<JS_TYPE extends PrimitiveType> {

// todo: remove. should not be on coretable
@imemo get maxTime(): Time {
return last(this.uniqTimesAsc) as Time
return max(this.allTimes) as Time
}

// todo: remove. should not be on coretable
@imemo get minTime(): Time {
return this.uniqTimesAsc[0]
return min(this.allTimes) as Time
}

// todo: remove? Should not be on CoreTable
Expand Down
21 changes: 17 additions & 4 deletions packages/@ourworldindata/core-table/src/CoreTableUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -379,11 +379,24 @@ describe(trimEmptyRows, () => {

describe(sortColumnStore, () => {
it("can sort", () => {
const result = sortColumnStore(
{ countries: ["usa", "can", "mex"], pops: [123, 21, 99] },
["pops"]
)
const columnStore = {
countries: ["usa", "can", "mex"],
pops: [123, 21, 99],
}
const result = sortColumnStore(columnStore, ["pops"])
expect(result["pops"]).toEqual([21, 99, 123])
expect(result["countries"]).toEqual(["can", "mex", "usa"])
expect(result).not.toBe(columnStore)
})

it("can detect a sorted array and leave it untouched", () => {
const columnStore = {
countries: ["usa", "can", "mex"],
pops: [21, 99, 123],
}
const result = sortColumnStore(columnStore, ["pops"])
expect(result["pops"]).toEqual([21, 99, 123])
expect(result).toBe(columnStore)
})
})

Expand Down
25 changes: 14 additions & 11 deletions packages/@ourworldindata/core-table/src/CoreTableUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -335,13 +335,15 @@ export function toleranceInterpolation(
}

// A dumb function for making a function that makes a key for a row given certain columns.
export const makeKeyFn =
(columnStore: CoreColumnStore, columnSlugs: ColumnSlug[]) =>
(rowIndex: number): string =>
export const makeKeyFn = (
columnStore: CoreColumnStore,
columnSlugs: ColumnSlug[]
): ((rowIndex: number) => string) => {
const cols = columnSlugs.map((slug) => columnStore[slug])
return (rowIndex: number): string =>
// toString() handles `undefined` and `null` values, which can be in the table.
columnSlugs
.map((slug) => toString(columnStore[slug][rowIndex]))
.join(" ")
cols.map((col) => toString(col[rowIndex])).join(" ")
}

const getColumnStoreLength = (store: CoreColumnStore): number => {
return max(Object.values(store).map((v) => v.length)) ?? 0
Expand Down Expand Up @@ -602,7 +604,7 @@ export const sortColumnStore = (
// Check if column store is already sorted (which is the case if newOrder is equal to range(0, startLen)).
// If it's not sorted, we will detect that within the first few iterations usually.
let isSorted = true
for (let i = 0; i <= len; i++) {
for (let i = 0; i < len; i++) {
if (newOrder[i] !== i) {
isSorted = false
break
Expand All @@ -622,14 +624,15 @@ const makeSortByFn = (
columnStore: CoreColumnStore,
columnSlugs: ColumnSlug[]
): ((indexA: number, indexB: number) => 1 | 0 | -1) => {
const numSlugs = columnSlugs.length
const cols = columnSlugs.map((slug) => columnStore[slug])

return (indexA: number, indexB: number): 1 | 0 | -1 => {
const nodeAFirst = -1
const nodeBFirst = 1

for (let slugIndex = 0; slugIndex < numSlugs; slugIndex++) {
const slug = columnSlugs[slugIndex]
const col = columnStore[slug]
// eslint-disable-next-line @typescript-eslint/prefer-for-of
for (let colIndex = 0; colIndex < cols.length; colIndex++) {
const col = cols[colIndex]
const av = col[indexA]
const bv = col[indexB]
if (av < bv) return nodeAFirst
Expand Down
13 changes: 6 additions & 7 deletions packages/@ourworldindata/core-table/src/OwidTable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
sumBy,
uniq,
sortNumeric,
last,
groupBy,
isNumber,
isEmpty,
Expand Down Expand Up @@ -100,10 +99,6 @@ export class OwidTable extends CoreTable<OwidRow, OwidColumnDef> {
) as Map<string, string>
}

@imemo get maxTime(): number | undefined {
return last(this.allTimes)
}

@imemo get entityIdColumn(): CoreColumn {
return (
this.getFirstColumnWithType(ColumnTypeNames.EntityId) ??
Expand All @@ -126,11 +121,15 @@ export class OwidTable extends CoreTable<OwidRow, OwidColumnDef> {
}

@imemo get minTime(): Time {
return this.allTimes[0]
return min(this.allTimes) as Time
}

@imemo get maxTime(): number | undefined {
return max(this.allTimes)
}

@imemo private get allTimes(): Time[] {
return this.sortedByTime.get(this.timeColumn.slug).values
return this.get(this.timeColumn.slug).values
}

@imemo get rowIndicesByEntityName(): Map<string, number[]> {
Expand Down
14 changes: 8 additions & 6 deletions packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
EPOCH_DATE,
OwidChartDimensionInterface,
OwidVariableType,
memoize,
} from "@ourworldindata/utils"
import { isContinentsVariableId } from "./GrapherConstants"

Expand Down Expand Up @@ -261,12 +262,13 @@ export const legacyToOwidTableAndDimensions = (
const daysColumn = variablesJoinedByDay.getColumns([
OwidTableSlugs.day,
])[0]
const yearsForDaysValues = []
for (const dayValue of daysColumn.values) {
yearsForDaysValues.push(
getYearFromISOStringAndDayOffset(EPOCH_DATE, dayValue as number)
)
}
const getYearFromISOStringMemoized = memoize((dayValue: number) =>
getYearFromISOStringAndDayOffset(EPOCH_DATE, dayValue)
)
const yearsForDaysValues = daysColumn.values.map((dayValue) =>
getYearFromISOStringMemoized(dayValue as number)
)

const newYearColumn = {
...daysColumn,
slug: OwidTableSlugs.year,
Expand Down