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

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

path: change `posix.join` to use array by HBSPS · Pull Request #54331 · nodejs/node

 

이번에 Node.js에 첫 기여를 해볼 수 있었다.

 

코드를 확인하던 중 아래와 같은 부분이 있었다.

/**
  * @param {...string} args
  * @returns {string}
  */
  join(...args) {
    if (args.length === 0)
      return '.';
    let joined;
    for (let i = 0; i < args.length; ++i) {
      const arg = args[i];
      validateString(arg, 'path');
      if (arg.length > 0) {
        if (joined === undefined)
          joined = arg;
        else
          joined += `/${arg}`;
      }
    }
    if (joined === undefined)
      return '.';
    return posix.normalize(joined);
  },

 

(path.join의 사용 방법에 대해서는 아래 문서를 참고하면 된다)

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

Path | Node.js v22.6.0 Documentation

 

개선점 찾기

위의 코드를 분석해보자.

1. 인자가 없다면 .을 반환한다.
2. (인자가 있다면) 각 args에 대해 빈 문자열이 아닌 경우 결과에 문자열을 더한다.
3. 연결된 문자열이 빈 문자열이라면 .을 반환한다.
4. (연결된 문자열이 있다면) normalize에 전달해 normalizing(상대 경로를 해결하고 경로 구분자를 os에 맞게 변환)한다.

위에서 해결할 수 있는 부분은 2번에 해당한다.

join('foo', '..', 'bar');

위와 같은 코드가 있다고 할 때 최종적으로 normalize 함수에 전달되는 인자는 다음과 같이 구성될 것이다.

'foo/../bar'

이를 통해 알 수 있는 것은 인자로 전달된 여러 경로 중 비어있지 않은 인자들을 /를 이용해 연결하는 것이다.

 

여기까지 해당하는 부분은 아래와 같다.

let joined;
for (let i = 0; i < args.length; ++i) {
  const arg = args[i];
  validateString(arg, 'path');
  if (arg.length > 0) {
    if (joined === undefined)
      joined = arg;
    else
      joined += `/${arg}`;
  }
}

joined가 undefined인 경우(값을 할당하지 않았으니 기본적으로 joined는 undefined로 초기화 됨) arg를 그냥 더하게 되며 이후 부터는 joined가 존재하기 때문에 /과 함께 arg를 더해주게 된다.

 

최종적으로 위와 같이 비어있는 문자열을 제외한 모든 args를 하나의 문자열로 만들게 되는 것이다.

(상대 경로를 해결하고 경로를 normalizing하는 것은 normalize 함수에서 수행하기 때문에 이 부분에서 자세히 알 필요는 없다)

 

개선 방법

위의 코드를 개선하기 위해 if문을 줄이려 시도했다.

 

joined는 한 번 문자열이 추가되면 다시 undefined가 되지 않는다.

 

따라서, joined가 한 번 변경된 뒤부터 해당 if문은 필요가 없게 되며 모든 arg가 해당 if문 안에 들어가기 때문에 불필요한 연산이라 생각했다.

 

이를 해결하기 위해 생각했던 방법은 배열을 활용하는 것이었다.

 

위의 형태에서 볼 수 있듯이 빈 문자열이 아닌 args를 모두 배열에 넣은 뒤 array.join을 사용한다면 쉽게 만들 수 있을 것으로 생각했다.

 

이를 적용하여 위의 코드를 수정하여 아래와 같이 작성하게 되었다.

/**
  * @param {...string} args
  * @returns {string}
  */
  join(...args) {
    if (args.length === 0)
      return '.';

    const path = [];
    for (let i = 0; i < args.length; ++i) {
      const arg = args[i];
      validateString(arg, 'path');
      if (arg.length > 0) {
        path.push(arg);
      }
    }

    if (path.length === 0)
      return '.';

    return posix.normalize(ArrayPrototypeJoin(path, '/'));
  },

 

 

primordials?

위의 코드를 보면 ArrayPrototypeJoin이라는 것을 볼 수 있다.

 

이것은 primordials라는 것중 하나로 이름에 알 수 있듯이 array.join과 같은 역할을 한다.

https://github.com/nodejs/node/blob/main/doc/contributing/primordials.md

node/doc/contributing/primordials.md at main · nodejs/node

 

그렇다면 Node.js에서 array.join 대신 위와 같이 복잡하게 작성하는 이유는 무엇일까?

const array = [1, 2, 3];
array.push(4); // Here `push` refers to %Array.prototype.push%.
console.log(JSON.stringify(array)); // [1,2,3,4]

// %Array.prototype%.push is modified in userland.
Array.prototype.push = function push(val) {
  return this.unshift(val);
};

array.push(5); // Now `push` refers to the modified method.
console.log(JSON.stringify(array)); // [5,1,2,3,4]

위와 같이 push 함수 등 JS 빌트인 객체의 경우 사용자가 직접 prototype을 수정하여 결과를 다르게 만들 수 있다.

(prototype pollution)

 

이를 방지하고자 Node.js에서는 primordials라는 것을 사용하여 프로토타입 오염에 대비하고 있다.

 

좋은 목적을 갖고 있지만 성능에서 아쉬운 부분도 있다.

• ArrayPrototypePush
• ArrayPrototypePop
• ArrayPrototypeShift
• ArrayPrototypeUnshift

여러가지 함수들이 있지만 대표적으로 (일반적으로 자주 사용하게 될) 위의 primordials의 경우는 성능에 이슈가 있다고 알려져 있다.

https://github.com/nodejs/node/blob/main/doc/contributing/primordials.md#primordials-with-known-performance-issues

node/doc/contributing/primordials.md at main · nodejs/node

따라서, 성능상 이점을 택할 것인지 안정성을 택할 것인지 고민해야 한다.

 

Node.js 내부적으로도 primordials에 대해 찬성하는 입장과 반대하는 입장이 공존하고 있다.

 

이에 대한 논의는 계속되고 있으며 과거 모든 JS 연산에 대해 기본적으로 적용하던 primordials를 성능과 가독성 등의 문제로 제거하는 PR도 볼 수 있다.

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

tls: remove prototype primordials by aduh95 · Pull Request #53699 · nodejs/node

 

이에 대해 현재 구체적인 primordials 사용의 구체적인 사용 사례를 도출하는 논의가 진행중이다.

https://github.com/nodejs/TSC/issues/1439

https://github.com/nodejs/primordials-use-cases

 

벤치마크

그렇다면 실제로 유의미한 성능의 차이가 있는지 확인해보자.

 

Node.js 디렉토리에는 benchmark라는 디렉토리가 있으며 주요한 기능들에 대한 벤치마크가 위치하게 된다.

 

위에서 수정한 코드는 path.join에 해당하며 그 중에서도 posix(리눅스 또는 맥)의 join만 해당한다.

 

일반적인 array.push를 사용한 경우 아래와 같은 결과가 나온다.

                                                               confidence improvement accuracy (*)   (**)  (***)
path/join-posix.js n=100000 paths='/foo|bar||baz/asdf|quux|..'        ***      5.95 %       ±1.42% ±1.90% ±2.49%

 

여기서 primordials에 해당하는 ArrayPrototypePush를 사용한 결과는 아래와 같다.

                                                               confidence improvement accuracy (*)   (**)  (***)
path/join-posix.js n=100000 paths='/foo|bar||baz/asdf|quux|..'                 0.20 %       ±0.63% ±0.84% ±1.09%

결과에서 확인할 수 있듯이 유의미한 성능의 차이가 있는 것을 볼 수 있다.

 

PR

최종적으로 PR에는 일반적인 push를 사용한 것을 올리게 되었다.

 

성능적으로 유의미한 차이가 있으며 ArrayPrototypePush를 사용한 경우 성능상에 이점이 없기 때문에 의미있는 개선이라 할 수 없을 것 같았다.

 

최근 primordials에 대한 논의가 계속되고 있으며 성능이 개선되었기 때문에 반대할 이유는 크게 없다고 생각했다.

(수정 요청이 들어오면 요구하는 방향에 맞추어 수정하는 것이 좋을 것이라 생각했다)

 

역시 primordials과 관련된 comment가 있었으며 최종적으로 이 코드에서는 필요가 없다는 확인을 받았다.

 

여담

Node.js에서 PR이 merge되기 위해서는 두 개 이상의 approve가 필요하다.

 

공교롭게도 이 PR에 approve를 준 두 사람이 각각 primordials 제거에 대해 제안하는 issue를 발행한 사람과 해당 issue에서 primordials의 효용성에 대해 주장하던 사람이라는 점이 흥미롭다.

https://github.com/nodejs/TSC/issues/1438

Removing `primordials` from Node.js project · Issue #1438 · nodejs/TSC

결론적으로 한 명은 primordials 제거에 찬성하고 다른 한 명은 primordials 제거에 반대하는 사람이라는 것이다.

 

해당 issue 이후 어떤 일이 있었길래 서로 상반된 생각을 가진 두 사람이 이 PR을 approve 했는지는 모르겠다.

 

오히려 primordials 제거에 반대한 사람이 먼저 approve를 눌렀다(?)

 

결과

결과적으로 두 개의 승인을 받고 Node.js main 브랜치에 merge 되었다.

(두 개의 approve를 받아도 PR이 열린지 최소 48시간이 있어야 merge 된다 - 해당 시간 동안 또 다른 의견 또는 논의가 있을 수 있기 때문)

 

내가 작성한 코드는 다음 릴리즈에 포함되어 모두가 사용할 수 있게 된다.

(개발자들이 path.join을 얼마나 사용할지는 모르겠다...)

 

이 과정을 통해 Node.js라는 대형 프로젝트에 기여하는 경험을 할 수 있었다.

 

Node.js 빌드, 수정, 벤치마크, 테스트, 커밋 메시지 형식, PR을 이용한 소통 등 기여를 위해 필요한 단계를 모두 경험할 수 있었다.

 

Node의 빌드는 생각보다 오래 걸리며 테스트는 수천개가 넘고 벤치마크를 통한 성능 측정까지 할 수 있도록 모두 준비되어 있었다.

 

모든 것들이 모두 오픈소스 기여자들이 만든 것임을 새삼 깨닫게 되었다.

 

아주 단순한 수정이었지만 성능에 변화가 있다는 사실에 놀랐으며 이 주제가 현재 Node.js 내부에서도 활발히 논의중인 내용이라는 것이 놀라웠다.

 

내가 작성한 코드를 남들이 모두 사용할 수 있음에 기대가 되는 한편 내가 작성한 코드를 남들이 모두 사용할 수 있기에 더 조심하게 되는 것 같다는 생각이 든다.