-
Notifications
You must be signed in to change notification settings - Fork 1
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.
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
[#61] 클라이언트에 js 엔진 올리기 #83
The head ref may contain hidden characters: "61-\uD074\uB77C\uC774\uC5B8\uD2B8\uC5D0-js-\uC5D4\uC9C4-\uC62C\uB9AC\uAE30"
Conversation
- quickjs-emscripten 이라는 라이브러리이다. - quickjs를 웹 어셈블리로 로드할 수 있게 하고 일부 간단한 ui를 추가한 라이브러리이다
const evalCode = (vm: QuickJSContext, code: string, params: string) => { | ||
const script = toRunableScript(code, params); | ||
|
||
const startTime = performance.now(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
별 게 아니긴 한데 궁금해서요! 29번째 줄과 31번째 줄 사이에 공백을 넣으신 이유가 있나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
가독성을 위해 논리가 바뀌는 영역을 분리한거에요.
동작시킬 스크립트 생성
스크립트 실행 및 성능측정
결과 반환
순서로 분리된 겁니다.
제 개인적인 분류 기준이라고 보면 됩니다. (사실 그래서 규칙성이 떨어져요)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM:D
@@ -10,7 +10,7 @@ self.addEventListener('message', async function (e: MessageEvent) { | |||
|
|||
try { | |||
const { code, param } = message; | |||
const result = evalCode(code, param); | |||
const result = await QuickJS.evaluate(code, param); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻 완벽한 모듈화네요
try { | ||
return evalCode(vm, code, params); | ||
} finally { | ||
vm.dispose(); | ||
runtime.dispose(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
evalCode가 정상작동하면 return하고 아니면 finally가 돌아가는 거 같은데,
에러가 발생했을 때 return이 안되고 finally가 돌아가서 메모리 비워주는 건가요??
정상작동 한다면 try 안에 return이 있어 finally가 호출이 안 되는 거 아닌가요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
와우 😎
const result = vm.unwrapResult(vm.evalCode(script)); | ||
const endTime = performance.now(); | ||
const value = vm.dump(result); | ||
result.dispose(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
정상적으로 돌아가면 여기까지 오게 되고 여기서 해제해주는 건가요?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네 그렇게 생각했는데 다시 보니까 엣지케이스가 좀 있네요
function solution() {
throw Error("err");
}
위와 같은 코드가 들어오면 dispose를 거치지 않게 되고 있습니다. 우선은 다른 메모리 할당해제가 잘 일어나서 누수가 없지 않을까? 생각되긴 하는데, 이건 그냥 추측이구요.
이런 케이스를 보완할 수 있는 방법을 좀 찾아보고 추가해볼게요
유저가 throw Error를 입력하면 모듈 밖에서도 throw하는 문제가 있었음 이 때문에 내부에서 throw를 한번 걸러줄 수 있게 코드를 수정 추가적으로 에러 코드를 출력하도록 변경함
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return 문 있을 때, try catch finally 실행 순서 잘 배우고 갑니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
한 일
현재 quickjs에선 마지막에 solution 함수를 실행하도록 하고 있습니다.
그리고 메모리 1GB, 스택 500MB를 기본적으로 사용하도록 설정하고 있습니다.
수정됨 (필독)
위와 같은 엣지 케이스에서 동작할 수 있게 코드를 수정했습니다.
참고
quickjs 관련 코드는 이해가 어려울 수 있습니다. 아래 문서를 참고해주세요
[기술 조사] quickjs-emscripten 사용법 간단 설명
QuickJS를 동작시키는 웹 어셈블리가 뭔지 모르겠다면
[기술 조사] 웹 어셈블리란 무엇인가? & 알고윗미에서의 활용 방안
QuickJS에 관한 문서들
[검증] QuickJS는 모든 브라우저에서 동일한 성능 보장할 수 있는가?
[검증] QuickJS는 모든 브라우저에서 동일한 Max Call Stack을 보장할 수 있는가?