Skip to content

added ok-computer #865

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

Merged
merged 4 commits into from
May 17, 2022
Merged

added ok-computer #865

merged 4 commits into from
May 17, 2022

Conversation

richardscarrott
Copy link
Contributor

@richardscarrott
Copy link
Contributor Author

richardscarrott commented May 17, 2022

Not sure why ok-computer is so slow as, although I haven't / don't intend to make micro optimisations, it's running significantly slower than others; I'll follow up with a new version once I've done some optimisation.

@moltar
Copy link
Owner

moltar commented May 17, 2022

@richardscarrott See #864, which might affect the benchmarks.

@hoeck has a PR for this in #866, which will get out soon, so might want to wait for that and retry your benchmarks.

Cheers and thanks for this PR!

Copy link
Owner

@moltar moltar left a comment

Choose a reason for hiding this comment

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

One small change though. We pin the exact version to make sure that we record the bench results against the exact version.

@moltar moltar self-requested a review May 17, 2022 17:10
@moltar moltar merged commit 1b47702 into moltar:master May 17, 2022
@richardscarrott
Copy link
Contributor Author

richardscarrott commented May 18, 2022

@moltar I did some profiling this morning and found it was genuinely slower than it should have been as it was hitting a catch block frequently. I made a fix here richardscarrott/ok-computer#7 and sent an update to the benchmarks here #869 -- something I wouldn't have prioritised if you hadn't made this project 👍

@moltar
Copy link
Owner

moltar commented May 18, 2022

Awesome!!

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