[Node.js 기여하기] 3. path 모듈 resolve 함수 성능 올리기

https://github.com/nodejs/node/pull/54835

path: remove repetitive conditional operator in `posix.resolve` by HBSPS · Pull Request #54835 · nodejs/node

 

저번과 유사하게 path 모듈의 성능을 올릴 수 있었다.

어떻게 보면 첫 번째 PR과 유사한 방식으로 개선한 것 같다.

resolve(...args) {
    let resolvedPath = '';
    let resolvedAbsolute = false;

    for (let i = args.length - 1; i >= -1 && !resolvedAbsolute; i--) {
      const path = i >= 0 ? args[i] : posixCwd();
      validateString(path, `paths[${i}]`);

      // Skip empty entries
      if (path.length === 0) {
        continue;
      }

      resolvedPath = `${path}/${resolvedPath}`;
      resolvedAbsolute =
        StringPrototypeCharCodeAt(path, 0) === CHAR_FORWARD_SLASH;
    }

    // At this point the path should be resolved to a full absolute path, but
    // handle relative paths to be safe (might happen when process.cwd() fails)

    // Normalize the path
    resolvedPath = normalizeString(resolvedPath, !resolvedAbsolute, '/',
                                   isPosixPathSeparator);

    if (resolvedAbsolute) {
      return `/${resolvedPath}`;
    }
    return resolvedPath.length > 0 ? resolvedPath : '.';
},

위의 함수는 path 모듈 중에서도 posix의 resolve 함수다.

 

해당 함수의 자세한 사용 방법은 아래 공식문서를 참고하면 된다.

https://nodejs.org/api/path.html#pathresolvepaths

Path | Node.js v22.8.0 Documentation

 

개선점 찾기

우선 위의 함수가 하는 역할에 대해 알아야 한다.

 

위의 함수는 join과 유사하게 동작하지만 인자로 전달된 경로 조각들 중 가장 마지막에 있는 절대경로를 기준으로 경로를 합치게 된다.

만약, 인자에 절대 경로가 포함되어 있지 않다면 process.cwd()를 기준으로 반환하게 된다.

for (let i = args.length - 1; i >= -1 && !resolvedAbsolute; i--) {
  const path = i >= 0 ? args[i] : posixCwd();
  validateString(path, `paths[${i}]`);

  // Skip empty entries
  if (path.length === 0) {
    continue;
  }

  resolvedPath = `${path}/${resolvedPath}`;
  resolvedAbsolute =
    StringPrototypeCharCodeAt(path, 0) === CHAR_FORWARD_SLASH;
}

위와 같이 for loop를 거꾸로 돌면서 가장 마지막의 절대 경로를 찾는 작업을 하게 된다.

 

만약, i가 -1이라면(인자 중 절대 경로가 포함되지 않은 경우) cwd를 기준으로 반환하게 된다.

 

그런데 위의 for loop에서 불필요한 부분이 있다.

 

path 변수는 조건에 args[i] 또는 posixCwd (실제로는 process.cwd()와 동일)로 설정된다.

 

하지만 posixCwd를 사용하는 경우는 모든 순회가 끝난 뒤 마지막 한 번 밖에 없다.

 

결론적으로 path 변수를 정하기 위해 삼항 연산자가 계속 실행될 필요가 없다는 것이다.

 

개선 방법

위의 코드를 개선하기 위해서는 불필요하게 반복되는 삼항 연산자를 제거하면 될 것이다.

 

최종적으로 아래 흐름과 같이 구성한다면 불필요한 연산을 줄일 수 있을 것으로 기대했다.

1. path 변수는 항상 args[i]가 된다.
2. 반복문은 args 갯수만큼 수행한다.
3. 반복문이 종료되었지만 절대 경로가 없는 경우 최종 경로 문자열 앞에 posixCwd를 추가한다.

위의 흐름을 코드로 나타내면 아래와 같다.

resolve(...args) {
    let resolvedPath = '';
    let resolvedAbsolute = false;

    for (let i = args.length - 1; i >= 0 && !resolvedAbsolute; i--) {
      const path = args[i];
      validateString(path, `paths[${i}]`);

      // Skip empty entries
      if (path.length === 0) {
        continue;
      }
      resolvedPath = `${path}/${resolvedPath}`;
      resolvedAbsolute =
        StringPrototypeCharCodeAt(path, 0) === CHAR_FORWARD_SLASH;
    }

    if (!resolvedAbsolute) {
      const cwd = posixCwd();
      resolvedPath = `${cwd}/${resolvedPath}`;
      resolvedAbsolute =
        StringPrototypeCharCodeAt(cwd, 0) === CHAR_FORWARD_SLASH;
    }

    // At this point the path should be resolved to a full absolute path, but
    // handle relative paths to be safe (might happen when process.cwd() fails)

    // Normalize the path
    resolvedPath = normalizeString(resolvedPath, !resolvedAbsolute, '/',
                                   isPosixPathSeparator);
    if (resolvedAbsolute) {
      return `/${resolvedPath}`;
    }
    return resolvedPath.length > 0 ? resolvedPath : '.';
},

최종적으로 path를 설정하기 위한 삼항 연산자를 제거하고 마지막에 절대 경로를 포함했는지 여부를 확인하여 posixCwd를 추가하도록 수정했다.

 

시행착오

if (!resolvedAbsolute) {
  const cwd = posixCwd();
  resolvedPath = `${cwd}/${resolvedPath}`;
  resolvedAbsolute =
    StringPrototypeCharCodeAt(cwd, 0) === CHAR_FORWARD_SLASH;
}

이 부분의 코드를 작성하며 시행착오를 겪었다.

 

위 코드에서 if문의 역할은 args 중 절대 경로가 없었다면 cwd를 resolvedPath에 추가하는 부분이다.

 

처음 위의 코드는 아래와 같은 형태였다.

if (!resolvedAbsolute) {
  const cwd = posixCwd();
  resolvedPath = `${cwd}/${resolvedPath}`;
  resolvedAbsolute = true;
}

posix에서 cwd는 절대 경로로 반환되기에 첫 글자를 확인할 필요 없이 resolvedAbsolute는 true가 된다고 생각했다.

(맞는 말이긴 하다. process.cwd()를 실행 한다면 /Users/어쩌구와 같이 반환되기 때문이다)

 

위와 같이 작성하고 테스트를 돌린 결과 한 테스트 케이스를 통과하지 못했다.

 

하나의 케이스만 통과하지 못했다면 수정하기 쉬울 것이다.

 

수정한 코드가 제대로 된 기능을 하지 못한다면 해당 함수를 사용하는 다른 부분의 테스트도 함께 통과하지 못하는 경우가 많기 때문이다.

 

만약 통과하지 못한 테스트가 일부이며 해당 모듈과 직접적으로 관련된 (해당 모듈에 대한) 테스트일 경우는 특수한 케이스에 대해 실패했다고 볼 수 있을 것이다.

 

통과하지 못한 테스트 코드를 찾아보면 아래와 같다.

if (!common.isWindows) {
  // Test handling relative paths to be safe when process.cwd() fails.
  process.cwd = () => '';
  assert.strictEqual(process.cwd(), '');
  const resolved = path.resolve();
  const expected = '.';
  assert.strictEqual(resolved, expected);
}

위 코드에서 마지막 assert를 통과하지 못했다.

 

통과하지 못한 이유를 분석해보면 의도적으로 process.cwd()의 반환 값을 빈 문자열로 변경했음을 알 수 있다.

 

처음과 같이 resolvedAbsolute를 단순히 true로 바꾸게 되면 위와 같이 process.cwd()가 기대한 반환값을 반환하지 못하는 경우에 문제가 될 수 있다.

 

결론적으로 process.cwd()를 사용하더라도 첫 번째 글자를 확인해야 한다.

 

여담

저번 primordials와 for...of에 이어 이번에도 pollution에 관한 내용이 포함되었다.

 

물론 pollution 관련 내용을 의도적으로 다루는 것은 아니지만 계속해서 관련된 내용이 나오는 것 같다.

 

이번 테스트 케이스를 통해 prototype pollution에 이어 함수의 반환값 자체를 바꿔버리는 것에 대해서도 대비를 하고 있는 것을 볼 수 있다.

 

사용자는 단순히 Node.js 안에서 원하는 기능을 꺼내 사용하기만 하면 되지만 이를 안정적으로 제공하기 위해 얼마나 많은 노력을 하고 있는지 알 수 있는 것 같다.

 

벤치마크

유의미한 개선이 있는지 확인하기 위해 벤치마크를 수행한 결과는 아래와 같다.

                                                                          confidence improvement accuracy (*)   (**)  (***)
path/resolve-posix.js n=100000 paths=''                                          ***      7.32 %       ±2.03% ±2.70% ±3.51%
path/resolve-posix.js n=100000 paths='|'                                         ***      6.23 %       ±0.74% ±0.98% ±1.28%
path/resolve-posix.js n=100000 paths='a/b/c/|../../..'                           ***      2.96 %       ±0.55% ±0.73% ±0.95%
path/resolve-posix.js n=100000 paths='foo/bar|/tmp/file/|..|a/../subfile'                -0.08 %       ±0.56% ±0.75% ±0.98%

(로컬)

 

                                                                          confidence improvement accuracy (*)   (**)  (***)
path/resolve-posix.js n=100000 paths=''                                          ***      9.61 %       ±1.14% ±1.51% ±1.97%
path/resolve-posix.js n=100000 paths='|'                                         ***      8.55 %       ±1.22% ±1.62% ±2.11%
path/resolve-posix.js n=100000 paths='a/b/c/|../../..'                           ***      3.15 %       ±1.04% ±1.39% ±1.82%
path/resolve-posix.js n=100000 paths='foo/bar|/tmp/file/|..|a/../subfile'                -0.39 %       ±0.90% ±1.20% ±1.57%

(Node.js Jenkins CI)

 

두 환경 모두에서 유의미한 성능의 차이가 나타났다.

 

PR

위의 벤치마크 결과를 바탕으로 PR을 올릴 수 있다.

벤치마크가 있는 함수 등을 수정하는 경우라면 벤치마크 결과가 가장 확실하게 이 코드를 설명할 수 있을 것이다.

(개인적으로 성능을 개선하는데 관심이 많아 성능의 개선을 쉽게 확인할 수 있는 벤치마크가 포함된 함수 개선을 선호하고 있다)

 

최종적으로는 세 개의 Approve를 받을 수 있었으며 로컬보다 Jenkins CI의 벤치마크 결과가 더 좋았다.

 

우여곡절

이 PR이 merge 되기까지 약 10일이 걸렸다.

 

기존 다른 PR에 비해 더 많은 시간이 걸렸으며 CI를 통과하지 못하는 경우도 있었다.

 

이것은 이 PR을 올릴 즈음에 Node.js Jenkins CI에서 문제가 발생했기 때문이다.

 

물론 내가 할 수 있는 것은 없으며 CI가 수정되어 누군가 CI를 다시 돌려주기를 기다리는 방법밖에 없다.

 

이 기간이 체감상 길게 느껴졌으며 코멘트를 통해 Approve를 준 사람들을 언급하는 등 이 PR에 다시 관심을 갖도록 하기 위해 노력했다.

최종적으로 CI가 고쳐졌는지 CI를 통과하고 merge 될 수 있었다.

 

결과

오랜 시간이 걸렸지만 PR이 머지되었다.

 

이것으로 세 번째 기여를 할 수 있었다.

 

의도치 않게 path 모듈에만 두 번의 성능 개선을 하게 되었다.

 

path만 하고자 하는 것은 아니었으나 path 모듈의 코드를 가장 오랫동안 봤기에 개선점을 찾기가 더 쉬웠던 것 같다.

 

Node.js의 성능을 올리는 것은 생각보다 재미있고 Node.js의 벤치마크가 잘 구성되어 있다는 것을 느낄 수 있었다.

 

지금 기여한 내용들은 22버전에 포함되어 있기 때문에 아직은 많은 사람들이 이 부분을 사용하지 않을 것이다.

 

하지만, 이후 lts 버전들에는 이 코드가 포함 될 것이고 시간이 지날수록 더 많은 사람들이 이 코드를 사용하게 될 것이다.

(물론 직접 이 코드를 사용할 일은 없고 모듈을 불러와서 사용하는 것은 여전할테지만)

 

누군가가 사용하는 함수의 성능을 올린다는 것에 보람을 느끼기도 하고 코드의 구조를 파악하는 과정이 어렵지만 재미있다.

 

블로그를 보면 Node.js path 모듈의 resolve와 join을 비교하는 글들이 있다.

 

비슷한듯 다른 결과를 내놓는 두 함수의 차이점은 다음과 같다.

• resolve: 가장 마지막에 있는 절대경로를 기준으로 인자를 합한 경로를 반환 (인자에 절대경로가 없다면 cwd를 기준으로 반환)
• join: 절대경로에 상관없이 왼쪽 인자부터 마지막 인자까지 합한 경로를 반환

이와 같은 내용의 새로운 블로그 글이 작성될 때 마다 읽어보는 재미도 있을 것 같다.

(내가 기여한 함수들과 관련된 블로그 글)

 

함수의 동작을 이해하는 가장 좋은 방법은 그 함수의 내부를 들여다 보는 것도 방법이 될 수 있다는 생각을 갖게 되었다.