Skip to content

⚡ Optimize activity event loop with Sakamoto's algorithm #295

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

⚡ Optimize activity event loop with Sakamoto's algorithm #295

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
20 changes: 13 additions & 7 deletions src/lib/github.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -707,20 +707,26 @@ export const fetchActivity = cache(async function fetchActivity(
);

const eventCountMap = new Map<string, number>();
const dayCache = new Map<string, number>();

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

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

medium

To maximize the performance benefits of this optimization, move the charCodeZero constant and the Sakamoto lookup table t outside of the loop. Defining them inside the loop causes unnecessary re-allocations and assignments in every iteration of the activity processing loop.

const charCodeZero = '0'.charCodeAt(0);
const t = [0, 3, 2, 5, 0, 3, 5, 1, 4, 6, 2, 4];

for (const event of allEvents) {
const createdAt = event.created_at;
const datePart = createdAt.slice(0, 10);

let day = dayCache.get(datePart);
if (day === undefined) {
day = new Date(datePart).getUTCDay(); // 0=Sun, 6=Sat
dayCache.set(datePart, day);
// Fast day extraction from YYYY-MM-DD
const charCodeZero = '0'.charCodeAt(0);
Comment on lines 709 to +715
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 22, 2026

Choose a reason for hiding this comment

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

P2 ループ内で charCodeZero を毎回計算している

'0'.charCodeAt(0) は常に 48 の不変値ですが、現在はループの各イテレーションで評価されています。ループの外に定数として巻き上げることで、無駄な関数呼び出しを排除できます。

Suggested change
const eventCountMap = new Map<string, number>();
const dayCache = new Map<string, number>();
for (const event of allEvents) {
const createdAt = event.created_at;
const datePart = createdAt.slice(0, 10);
let day = dayCache.get(datePart);
if (day === undefined) {
day = new Date(datePart).getUTCDay(); // 0=Sun, 6=Sat
dayCache.set(datePart, day);
// Fast day extraction from YYYY-MM-DD
const charCodeZero = '0'.charCodeAt(0);
const eventCountMap = new Map<string, number>();
const charCodeZero = '0'.charCodeAt(0);
const sakamotoT = [0, 3, 2, 5, 0, 3, 5, 1, 4, 6, 2, 4];
for (const event of allEvents) {
const createdAt = event.created_at;
// Fast day extraction from YYYY-MM-DD
Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/lib/github.ts
Line: 709-715

Comment:
**ループ内で `charCodeZero` を毎回計算している**

`'0'.charCodeAt(0)` は常に `48` の不変値ですが、現在はループの各イテレーションで評価されています。ループの外に定数として巻き上げることで、無駄な関数呼び出しを排除できます。

```suggestion
  const eventCountMap = new Map<string, number>();
  const charCodeZero = '0'.charCodeAt(0);
  const sakamotoT = [0, 3, 2, 5, 0, 3, 5, 1, 4, 6, 2, 4];

  for (const event of allEvents) {
    const createdAt = event.created_at;

    // Fast day extraction from YYYY-MM-DD
```

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

const y = (createdAt.charCodeAt(0) - charCodeZero) * 1000 + (createdAt.charCodeAt(1) - charCodeZero) * 100 + (createdAt.charCodeAt(2) - charCodeZero) * 10 + (createdAt.charCodeAt(3) - charCodeZero);
const m = (createdAt.charCodeAt(5) - charCodeZero) * 10 + (createdAt.charCodeAt(6) - charCodeZero);
const d = (createdAt.charCodeAt(8) - charCodeZero) * 10 + (createdAt.charCodeAt(9) - charCodeZero);

// Sakamoto's algorithm for day of week
const t = [0, 3, 2, 5, 0, 3, 5, 1, 4, 6, 2, 4];
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 22, 2026

Choose a reason for hiding this comment

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

P1 ループ内での配列アロケーション — 最適化の逆効果

const t = [0, 3, 2, 5, 0, 3, 5, 1, 4, 6, 2, 4] がループの内側で毎イテレーション新規に 12 要素配列を生成しています。PR 説明で「中間メモリのガーベジをほぼ排除」と謳っているにもかかわらず、100,000 回のイベント処理では同数の配列オブジェクトが生まれ続けます。dayCache の Map オーバーヘッドを除去した恩恵をほぼ相殺してしまいます。t はループの外側（あるいは fetchActivity 関数スコープの外）で定数として定義すべきです。

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/lib/github.ts
Line: 721

Comment:
**ループ内での配列アロケーション — 最適化の逆効果**

`const t = [0, 3, 2, 5, 0, 3, 5, 1, 4, 6, 2, 4]` がループの内側で毎イテレーション新規に 12 要素配列を生成しています。PR 説明で「中間メモリのガーベジをほぼ排除」と謳っているにもかかわらず、100,000 回のイベント処理では同数の配列オブジェクトが生まれ続けます。`dayCache` の `Map` オーバーヘッドを除去した恩恵をほぼ相殺してしまいます。`t` はループの外側（あるいは `fetchActivity` 関数スコープの外）で定数として定義すべきです。

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

let sy = y;
if (m < 3) {
sy -= 1;
}
const day = (sy + Math.floor(sy/4) - Math.floor(sy/100) + Math.floor(sy/400) + t[m-1] + d) % 7;
Comment on lines +715 to +726
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 22, 2026

Choose a reason for hiding this comment

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

medium

With the constants moved outside the loop, the logic here can be simplified. Additionally, per repository rules, manually parsed numeric values like the month m must be validated before being used as array indices to prevent out-of-bounds access to the lookup table t.

Suggested change
const charCodeZero = '0'.charCodeAt(0);
const y = (createdAt.charCodeAt(0) - charCodeZero) * 1000 + (createdAt.charCodeAt(1) - charCodeZero) * 100 + (createdAt.charCodeAt(2) - charCodeZero) * 10 + (createdAt.charCodeAt(3) - charCodeZero);
const m = (createdAt.charCodeAt(5) - charCodeZero) * 10 + (createdAt.charCodeAt(6) - charCodeZero);
const d = (createdAt.charCodeAt(8) - charCodeZero) * 10 + (createdAt.charCodeAt(9) - charCodeZero);
// Sakamoto's algorithm for day of week
const t = [0, 3, 2, 5, 0, 3, 5, 1, 4, 6, 2, 4];
let sy = y;
if (m < 3) {
sy -= 1;
}
const day = (sy + Math.floor(sy/4) - Math.floor(sy/100) + Math.floor(sy/400) + t[m-1] + d) % 7;
const y = (createdAt.charCodeAt(0) - charCodeZero) * 1000 + (createdAt.charCodeAt(1) - charCodeZero) * 100 + (createdAt.charCodeAt(2) - charCodeZero) * 10 + (createdAt.charCodeAt(3) - charCodeZero);
const m = (createdAt.charCodeAt(5) - charCodeZero) * 10 + (createdAt.charCodeAt(6) - charCodeZero);
const d = (createdAt.charCodeAt(8) - charCodeZero) * 10 + (createdAt.charCodeAt(9) - charCodeZero);
// Sakamoto's algorithm for day of week
let sy = y;
if (m < 3) {
sy -= 1;
}
if (m < 1 || m > 12) throw new Error('Invalid month');
const day = (sy + Math.floor(sy / 4) - Math.floor(sy / 100) + Math.floor(sy / 400) + t[m - 1] + d) % 7;
References
  1. Validate manually parsed numeric values before using them as array indices to prevent out-of-bounds access and data corruption.


// Fast hour extraction from YYYY-MM-DDTHH:MM:SSZ
const charCodeZero = '0'.charCodeAt(0);
// charCodeZero is already defined above
const hour = (createdAt.charCodeAt(11) - charCodeZero) * 10 + (createdAt.charCodeAt(12) - charCodeZero);
heatmap[day][hour]++;

Expand Down
Loading