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.

Sign up for GitHub

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

Jump to bottom

Put out benchmark fires #404

Merged
merged 3 commits into from
Oct 12, 2023
Merged

Put out benchmark fires #404

merged 3 commits into from
Oct 12, 2023

Conversation

kazimuth
Copy link
Contributor

@kazimuth kazimuth commented Oct 10, 2023 •
edited
Loading

Attempting to address #401 #402 #403

Centril
Centril reviewed Oct 10, 2023
View reviewed changes
crates/bench/benches/generic.rs Outdated Show resolved Hide resolved
crates/bench/benches/generic.rs Outdated Show resolved Hide resolved
@kazimuth kazimuth force-pushed the kazimuth/benchfires branch from db119df to a5b50f4 Compare October 10, 2023 19:40
@kazimuth
Copy link
Contributor Author

kazimuth commented Oct 10, 2023 •
edited
Loading

@Centril, fixed the issue you pointed out. @mamcx, I've tried to revert as little of your commit as possible -- only the stuff in the bench crate. We can add in the benches for spacetime sql tomorrow.

Waiting on https://github.com/clockworklabs/SpacetimeDB/actions/runs/6473567337/job/17576553107 ...
Ah: it works, but it also runs the previous commit, which fails because the benchmarks weren't working there. The part that we care about is now working there.

#403 and #402 should be fixed, #401 I still have no idea about.

@kazimuth kazimuth enabled auto-merge (squash) October 10, 2023 20:24
@kazimuth
Copy link
Contributor Author

Ok, now this is ready for merge. I still haven't fixed the recompilation issue (#401) but I'll leave that for another day.

@kazimuth kazimuth requested a review from drogus October 11, 2023 18:41
RReverser
RReverser approved these changes Oct 12, 2023
View reviewed changes
Copy link
Member

@RReverser RReverser left a comment

Choose a reason for hiding this comment

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

I'm going to just go ahead and merge this as I want to unblock benchmarks on branches / PRs.

@kazimuth kazimuth merged commit cd88c21 into master Oct 12, 2023
5 of 6 checks passed
@RReverser
Copy link
Member

Looks like it didn't address #402: https://github.com/clockworklabs/SpacetimeDB/actions/runs/6496776998/job/17644465697

@RReverser
Copy link
Member

Ah probably because there's pre-existing cache without those newly declared files. Might be worth cleaning cache manually.

@RReverser
Copy link
Member

Went through caches and deleted all those from master; let's see if that helps...

@RReverser
Copy link
Member

Nope somehow still same error even when it clearly says "cache not found": https://github.com/clockworklabs/SpacetimeDB/actions/runs/6497076599/job/17645385541

@drogus drogus deleted the kazimuth/benchfires branch October 13, 2023 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Centril Centril Centril left review comments

@RReverser RReverser RReverser approved these changes

@drogus drogus Awaiting requested review from drogus

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kazimuth @RReverser @Centril