Skip to content

Conversation

@yaacovCR
Copy link
Contributor

@yaacovCR yaacovCR commented Jun 7, 2022

...to try to avoid involuntary context switches.

This seems to me to be pretty TETAW ("too easy to actually work").

@netlify
Copy link

netlify bot commented Jun 7, 2022

Deploy Preview for compassionate-pike-271cb3 ready!

Name Link
🔨 Latest commit e7fc7dd
🔍 Latest deploy log https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/62a0ab9f7137bb000819f989
😎 Deploy Preview https://deploy-preview-3633--compassionate-pike-271cb3.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link

github-actions bot commented Jun 7, 2022

Hi @yaacovCR, I'm @github-actions bot happy to help you with this PR 👋

Supported commands

Please post this commands in separate comments and only one per comment:

  • @github-actions run-benchmark - Run benchmark comparing base and merge commits for this PR
  • @github-actions publish-pr-on-npm - Build package from this PR and publish it on NPM

@yaacovCR

This comment has been minimized.

@github-actions
Copy link

github-actions bot commented Jun 7, 2022

@github-actions run-benchmark

@yaacovCR Something went wrong, please check log.

@yaacovCR
Copy link
Contributor Author

yaacovCR commented Jun 7, 2022

https://github.com/graphql/graphql-js/runs/6779810535?check_suite_focus=true#step:6:17

SystemError [ERR_SYSTEM_ERROR]: A system error occurred: uv_os_setpriority returned EACCES (permission denied)
at new SystemError (node:internal/errors:239:5)
at new NodeError (node:internal/errors:342:7)
at Object.setPriority (node:os:305:11)
at file:///home/runner/work/graphql-js/graphql-js/[eval1]:7:8
at ModuleJob.run (node:internal/modules/esm/module_job:198:25)
at async ESMLoader.eval (node:internal/modules/esm/loader:252:24)
at async loadESM (node:internal/process/esm_loader:85:5)
at async handleMainPromise (node:internal/modules/run_main:61:12) {
code: 'ERR_SYSTEM_ERROR',
info: {
errno: -13,
code: 'EACCES',
message: 'permission denied',
syscall: 'uv_os_setpriority'
},
errno: [Getter/Setter],
syscall: [Getter/Setter]
}

@yaacovCR yaacovCR closed this Jun 7, 2022
@yaacovCR yaacovCR deleted the set-priority branch June 7, 2022 18:00
@yaacovCR yaacovCR restored the set-priority branch June 7, 2022 18:07
@yaacovCR
Copy link
Contributor Author

yaacovCR commented Jun 7, 2022

Not sure if sudo can help us here. if i were github actions, it wouldn't!

@yaacovCR yaacovCR reopened this Jun 7, 2022
@yaacovCR yaacovCR closed this Jun 7, 2022
@yaacovCR yaacovCR reopened this Jun 7, 2022
@graphql graphql deleted a comment from github-actions bot Jun 7, 2022
@graphql graphql deleted a comment from github-actions bot Jun 7, 2022
@graphql graphql deleted a comment from github-actions bot Jun 7, 2022
@graphql graphql deleted a comment from github-actions bot Jun 7, 2022
@graphql graphql deleted a comment from github-actions bot Jun 7, 2022
@IvanGoncharov
Copy link
Member

@yaacovCR I tried it locally and it didn't work.
But we can try on CI, maybe it will work there.
Also, CPU affinity can help, since the GH runner has 2 CPUs we can bound the benchmark to the second one.

@yaacovCR
Copy link
Contributor Author

yaacovCR commented Jun 7, 2022

sudo and setPriority seem to be working! although another erupts when writing for some reason...

I can read more about affinity

@yaacovCR
Copy link
Contributor Author

yaacovCR commented Jun 7, 2022

I am guessing when using a shell, piping via 3 does not work

fs.writeFileSync(3, JSON.stringify(result));

@IvanGoncharov
Copy link
Member

@IvanGoncharov
Copy link
Member

IvanGoncharov commented Jun 7, 2022

@yaacovCR Simpler solution would be to run the entire benchmark under sudo.

@yaacovCR
Copy link
Contributor Author

yaacovCR commented Jun 7, 2022

@yaacovCR Simpler solution would be to run the entire benchmark under sudo.

That works for me.

As a side note, I want to make sure this works cross platform. I would assume that ci would add sudo to the whole command and local users would be required to do so on their own or use the windows equivalent.

to try to avoid involuntary context switches
@graphql graphql deleted a comment from github-actions bot Jun 8, 2022
@yaacovCR

This comment has been minimized.

@github-actions
Copy link

github-actions bot commented Jun 8, 2022

@github-actions run-benchmark

@yaacovCR Something went wrong, please check log.

@yaacovCR

This comment has been minimized.

@github-actions
Copy link

github-actions bot commented Jun 8, 2022

@github-actions run-benchmark

@yaacovCR Something went wrong, please check log.

@IvanGoncharov sudo on the benchmark was not working did not seem to pass through
@yaacovCR

This comment has been minimized.

@github-actions
Copy link

github-actions bot commented Jun 8, 2022

@github-actions run-benchmark

@yaacovCR Something went wrong, please check log.

@yaacovCR
Copy link
Contributor Author

yaacovCR commented Jun 8, 2022

@IvanGoncharov

sudo: you are not permitted to use the -C option

Needs further research

@IvanGoncharov
Copy link
Member

Needs further research

@yaacovCR Can I quickly try to run some experiments on top of your PR?

@yaacovCR
Copy link
Contributor Author

yaacovCR commented Jun 8, 2022

Needs further research

@yaacovCR Can I quickly try to run some experiments on top of your PR?

Definitely. Good luck!

@yaacovCR
Copy link
Contributor Author

yaacovCR commented Oct 1, 2024

Closing for now. Would be on wishlist to revisit in terms of our overall benchmarking picture.

@yaacovCR yaacovCR closed this Oct 1, 2024
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.

2 participants