Skip to content

⚡ perf: Optimize 2D array allocation in heatmap generation #449

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.

Sign up for GitHub

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

Open
is0692vs wants to merge 1 commit into
base: main
Choose a base branch
from
Open

⚡ perf: Optimize 2D array allocation in heatmap generation #449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 13 additions & 2 deletions src/lib/githubYearInReview.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,17 @@ import { buildHourlyHeatmapFromCommitDates, getMostActiveDayFromCalendar, getMos
import { logger } from "@/lib/logger";



const EMPTY_HEATMAP = [
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
];
Comment on lines +10 to +18

@gemini-code-assist gemini-code-assist Bot Jul 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Returning a shared global mutable array (EMPTY_HEATMAP) from fetchCommitActivityHeatmap is highly dangerous, especially in a server-side environment (server-only). If any caller or consumer mutates the returned heatmap, it will pollute the shared EMPTY_HEATMAP instance across all subsequent requests and users.

To prevent cross-request state pollution while keeping the performance benefits of static array literals, we should define a helper function createEmptyHeatmap that returns a fresh array literal on each call. Ensure the function has an explicit return type to maintain type safety.

Suggested change
const EMPTY_HEATMAP = [
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
];
const createEmptyHeatmap = (): number[][] => [
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
];
References
  1. Maintain explicit return types for functions in TypeScript to ensure type safety and API clarity.


Comment on lines +9 to +19

@greptile-apps greptile-apps Bot Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 共有ミュータブル定数を参照のまま返している問題

EMPTY_HEATMAP はモジュールレベルで定義された共有オブジェクトですが、return EMPTY_HEATMAP で同一の参照をそのまま返しています。Next.js のサーバー環境ではモジュールレベルの変数はリクエスト間で共有されるため、呼び出し元が返り値の配列を変更した場合（例: heatmap[0][0] = 99）、以降のすべてのエラーレスポンスが汚染されたデータを返すサイレントなデータ破損が発生します。現在の route.ts では NextResponse.json() でシリアライズするだけなので問題は顕在化していませんが、将来の変更で簡単に破壊される脆弱な設計です。

return EMPTY_HEATMAP の代わりに return EMPTY_HEATMAP.map(row => [...row]) でディープコピーを返すか、モジュール定数を廃止してその場で新たな配列を生成する方式に戻すことを推奨します。

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/lib/githubYearInReview.ts
Line: 9-19

Comment:
**共有ミュータブル定数を参照のまま返している問題**

`EMPTY_HEATMAP` はモジュールレベルで定義された共有オブジェクトですが、`return EMPTY_HEATMAP` で同一の参照をそのまま返しています。Next.js のサーバー環境ではモジュールレベルの変数はリクエスト間で共有されるため、呼び出し元が返り値の配列を変更した場合（例: `heatmap[0][0] = 99`）、以降のすべてのエラーレスポンスが汚染されたデータを返すサイレントなデータ破損が発生します。現在の `route.ts` では `NextResponse.json()` でシリアライズするだけなので問題は顕在化していませんが、将来の変更で簡単に破壊される脆弱な設計です。

`return EMPTY_HEATMAP` の代わりに `return EMPTY_HEATMAP.map(row => [...row])` でディープコピーを返すか、モジュール定数を廃止してその場で新たな配列を生成する方式に戻すことを推奨します。

How can I resolve this? If you propose a fix, please make it concise.

const YEAR_IN_REVIEW_QUERY = `query($login: String!, $from: DateTime!, $to: DateTime!, $maxRepositories: Int!) {
user(login: $login) {
id
Expand Down Expand Up @@ -306,7 +317,7 @@ export async function fetchCommitActivityHeatmap(username: string, year: number,
const topRepository = mergeTopRepository(reposResponse.user.contributionsCollection);

if (!topRepository) {
return Array.from({ length: 7 }, () => Array.from({ length: 24 }, () => 0));
return EMPTY_HEATMAP;

@gemini-code-assist gemini-code-assist Bot Jul 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

To avoid returning a shared mutable reference, call the createEmptyHeatmap helper function to return a fresh array instance.

Suggested change
return EMPTY_HEATMAP;
return createEmptyHeatmap();

}

const [owner, repo] = topRepository.name.split("/");
Expand All @@ -321,7 +332,7 @@ export async function fetchCommitActivityHeatmap(username: string, year: number,
handleRateLimit(res);
}
if (!res.ok) {
return Array.from({ length: 7 }, () => Array.from({ length: 24 }, () => 0));
return EMPTY_HEATMAP;

@gemini-code-assist gemini-code-assist Bot Jul 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

To avoid returning a shared mutable reference, call the createEmptyHeatmap helper function to return a fresh array instance.

Suggested change
return EMPTY_HEATMAP;
return createEmptyHeatmap();

}

const commits = (await res.json()) as GitHubCommit[];
Expand Down
10 changes: 9 additions & 1 deletion src/lib/yearInReviewUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,15 @@
* @returns A 2D array representing the heatmap [day][hour].
*/
export function buildHourlyHeatmapFromCommitDates(commitDates: string[]): number[][] {
const heatmap = Array.from({ length: 7 }, () => Array.from({ length: 24 }, () => 0));
const heatmap = [
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
];

// Performance Optimization: Cache weekday calculations to avoid expensive Date parsing in the loop
const dayCache = new Map<string, number>();
Expand Down
Loading