Fix/optimized closed tile: do not use worker just using parallel by kimgh06 · Pull Request #53 · gamultong/gamulpung-client

kimgh06 · 2025-10-16T12:29:32Z

good

…lay component - Changed project name from "gamultong" to "gamulpung". - Removed WebGPU imports and related logic, switching to CPU processing only for tile handling. - Enhanced tile processing efficiency by implementing a fast hex character lookup and optimizing tile parsing logic. - Updated throttling mechanism for tile requests to prevent excessive server calls.

- Added font preloading for improved performance in RootLayout. - Introduced cleanup functions for canvas and font resources in CanvasRenderComponent to prevent memory leaks. - Implemented timer management in Play component to ensure proper cleanup on unmount. - Enhanced tilemap component with resource cleanup on unmount to manage textures and sprites effectively.

…Canvas components - Implemented SIMD-style optimizations for tile processing in the Play component, enhancing performance through batch processing and Web Worker integration. - Added fallback mechanisms for tile processing to ensure reliability when the Web Worker fails. - Refactored cursor direction and color constants to improve consistency across components, replacing direct references with imported constants. - Enhanced tilemap rendering logic by streamlining cursor color handling and texture management.

…Tilemap components - Introduced a vectorized lookup table (LUT) for efficient tile type determination based on 16-bit combinations, improving tile processing speed. - Implemented asynchronous tile replacement using SharedArrayBuffer and Atomics for optimal parallel processing, enhancing performance on multi-core systems. - Added concurrency control for texture creation in the Tilemap component, allowing for efficient resource management during rendering. - Improved texture caching and readiness state management to ensure smooth rendering of tiles and numbers.

…WebGPU logic - Removed WebGPU initialization and related comments, transitioning to CPU processing only. - Updated variable names for clarity, changing ZOOM_MIN to ANIMATION_ZOOM_MIN in CanvasRenderComponent. - Enhanced cleanup logic in Play component to ensure proper resource management on unmount.

- Simplified the limit function for task execution, enhancing readability and error handling. - Replaced for loops with forEach for flag texture generation, improving code clarity. - Updated variable names for consistency in tile processing, ensuring better maintainability. - Enhanced the isClosedOrFlag function for improved readability and performance. - Optimized numeric key generation for textures, ensuring clarity in tile indexing.

Copilot

Pull Request Overview

This PR optimizes tile rendering by removing web worker overhead and implementing CPU-based parallel processing with vectorized lookup tables. The main focus is on improving performance through direct parallel processing instead of worker-based approaches.

Renamed constants to uppercase naming convention (CURSOR_COLORS, CURSOR_DIRECTIONS, OTHER_CURSOR_COLORS)
Replaced web worker tile parsing with CPU-based parallel processing using Promise.all and optional SharedArrayBuffer
Implemented vectorized lookup tables (LUT) for faster tile type resolution
Added proper resource cleanup for canvas contexts, textures, and timers

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/constants/cursor.ts	Renamed exported constants to uppercase convention
src/components/tilemap/index.tsx	Refactored texture generation to use parallel processing with concurrency limiting and added cleanup hooks
src/components/canvas/index.tsx	Updated constant references and added cleanup functions for canvas and font resources
src/app/play/page.tsx	Removed web worker implementation, added vectorized LUT processing, implemented parallel tile processing with SharedArrayBuffer support
src/app/layout.tsx	Added font preloading links for performance optimization
package.json	Changed package name from "gamultong" to "gamulpung"

Comments suppressed due to low confidence (5)

src/app/play/page.tsx:1

Korean comment 'Canvas 컨텍스트 정리' should be translated to English for consistency. Consider 'Cleanup canvas contexts' instead.

'use client';

src/app/play/page.tsx:1

Korean comment '벡터 에셋 정리' should be translated to English for consistency. Consider 'Cleanup vector assets' instead.

'use client';

src/app/play/page.tsx:1

Korean comments should be translated to English. Also, the if block does nothing and should be removed or properly implemented if font cleanup is needed.

'use client';

src/app/play/page.tsx:1

Korean comment should be translated to English. Consider: 'Vectorized LUT: Pre-compute all 16-bit combinations (64KB)'

'use client';

src/app/play/page.tsx:1

Korean comment should be translated to English. Consider: 'Check if font is already loaded'

'use client';

Copilot · 2025-10-16T12:31:50Z

+  const createLimiter = (maxConcurrent: number) => {
+    let activeCount = 0;
+    const queue: Array<() => void> = [];
+    const runNext = () => {
+      if (activeCount >= maxConcurrent) return;
+      const nextTask = queue.shift();
+      if (!nextTask) return;
+      activeCount++;
+      nextTask();
+    };
+    return <T,>(task: () => Promise<T>): Promise<T> =>
+      new Promise<T>((resolve, reject) => {
+        const execute = () => {
+          task()
+            .then(resolve)
+            .catch(reject)
+            .finally(() => {
+              activeCount--;
+              runNext();
+            });
+        };
+        if (activeCount < maxConcurrent) {
+          activeCount++;
+          execute();
+        } else queue.push(execute);
+      });
+  };


The createLimiter function is defined inside the component body, meaning it will be recreated on every render. This should be moved outside the component or memoized with useCallback to avoid unnecessary recreations.

Copilot · 2025-10-16T12:31:51Z

+  const isClosedOrFlag = (t: string) => [TileContent.CLOSED, TileContent.FLAGGED].some(f => f === t);

  const getTileTexturesForContent = (content: string | number, defaults: { outerTexture?: Texture; innerTexture?: Texture }) => {
+    if (!content) return { ...defaults, closed: true };


The function returns closed: true for empty content, but this may incorrectly treat missing tiles as closed tiles. Consider returning closed: false or a distinct state to differentiate between empty and closed tiles.

Suggested change

if (!content) return { ...defaults, closed: true };

if (!content) return { ...defaults, closed: false };

Copilot · 2025-10-16T12:31:51Z

        const { width, height } = texture;
        const scale = { x: w / width, y: h / height };
-        textElements[textIdx++] = cloneElement(baseNum, { key: typeKeyBase + 4, x: startX + tileSize / 2, y: startY + tileSize / 2, scale });
+        textElements[textIdx++] = cloneElement(baseNum, { key: key + 4, x: startX + tileSize / 2, y: startY + tileSize / 2, scale });


The magic number 4 is added to the key without explanation. Consider extracting this as a named constant like NUMERIC_KEY_OFFSET to clarify its purpose.

Copilot · 2025-10-16T12:31:51Z

    const now = performance.now();
-    if (type === Direction.ALL && now - requestedTilesTimeRef.current < 0.3) return;
+    // Throttle ALL-tiles requests to avoid spamming server (300 ms window)
+    if (type === Direction.ALL && now - requestedTilesTimeRef.current < 300) return;


The magic number 300 represents milliseconds but lacks clarity. Consider extracting it as a named constant like TILE_REQUEST_THROTTLE_MS to improve code readability.

Copilot · 2025-10-16T12:31:51Z

+    const cpuCores = navigator.hardwareConcurrency || 4;
+
+    // CPU 코어 수에 맞춰 최적 워커 수 결정
+    const workerCount = Math.min(cpuCores, Math.ceil(totalTiles / 32)); // 최소 32개 타일당 1워커


The magic number 32 represents tiles per worker but is not documented. Consider extracting it as a named constant like MIN_TILES_PER_WORKER with a comment explaining the threshold choice.

kimgh06 added 6 commits October 15, 2025 21:56

Copilot AI review requested due to automatic review settings October 16, 2025 12:29

kimgh06 merged commit 334b25e into main Oct 16, 2025
1 check passed

Copilot AI reviewed Oct 16, 2025

View reviewed changes

kimgh06 deleted the fix/optimizedClosedTile branch December 23, 2025 12:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix/optimized closed tile: do not use worker just using parallel#53

Fix/optimized closed tile: do not use worker just using parallel#53
kimgh06 merged 6 commits into
mainfrom
fix/optimizedClosedTile

kimgh06 commented Oct 16, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 16, 2025

Uh oh!

Copilot AI Oct 16, 2025

Uh oh!

Copilot AI Oct 16, 2025

Uh oh!

Copilot AI Oct 16, 2025

Uh oh!

Copilot AI Oct 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	if (!content) return { ...defaults, closed: true };
	if (!content) return { ...defaults, closed: false };

Uh oh!

Conversation

kimgh06 commented Oct 16, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants