Skip to content
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

perf(ops): optimize permission check #11800

Merged
merged 3 commits into from
Sep 22, 2021

Conversation

AaronO
Copy link
Contributor

@AaronO AaronO commented Aug 21, 2021

Removes the overhead of permission check (logging) on access granted (common case):

Delta measured on perf_now from deno_common bench:

... Before ...
perf_now:            	n = 500000, dt = 0.264s, r = 1893939/s, t = 528ns/op
... After ...
perf_now:            	n = 500000, dt = 0.083s, r = 6024096/s, t = 166ns/op

So ~3x faster on op_now (which is a low-overhead op)

Why is this faster ?

Avoids "expensive" format calls if no debug logging is enabled

Removes the overhead of permission check on access granted (should be common case):

Delta measured on `perf_now` from `deno_common` bench:
- before: `528ns/op
- after: `166ns/op`

So ~3x faster
@MierenManz
Copy link

@AaronO i think you may got the before and after values wrong. Unless it got about 3x slower 😅

@AaronO
Copy link
Contributor Author

AaronO commented Aug 21, 2021

@AaronO i think you may got the before and after values wrong. Unless it got about 3x slower 😅

Good catch !

Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AaronO AaronO merged commit a33ee08 into denoland:main Sep 22, 2021
@AaronO AaronO deleted the perf/ops-permission-check branch September 22, 2021 22:46
caspervonb pushed a commit to caspervonb/deno that referenced this pull request Sep 26, 2021
* perf(ops): optimize permission check

Removes the overhead of permission check on access granted (should be common case):

Delta measured on `perf_now` from `deno_common` bench:
- before: `528ns/op
- after: `166ns/op`

So ~3x faster
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants