-
Notifications
You must be signed in to change notification settings - Fork 19
doxbee-async and doxbee-promise: add side-effects #247
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for webkit-jetstream-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
I could not really observe any score changes (might be worth rechecking): After: |
|
I suggest doing similar in |
|
Weirdly enough I start to see perf differences now
|
| } | ||
| async function dummy_2(a) { | ||
| Queryish.count++; | ||
| } |
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.
They should avoid using async, and do
function dummy_1() {
Queryish.count++;
return Promise.resolve(undefined);
}
function dummy_2(a) {
Queryish.count++;
return Promise.resolve(undefined);
}
because that's the purpose of this doxbee-promise (not async, that's tested in doxbee-async. testing chain of promises).
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.
I'm still observing quite a diff even with this fixed for JSC (almost none for V8):
Before: 321.0 ± 11.0(3.35%)
After: 309.0 ± 13.0(4.24%)
Add Queryish.count and use it as simple side-effect in the dummy functions.