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

Improve condvar signaling #323

Merged
merged 1 commit into from
Dec 4, 2024
Merged

Conversation

krtab
Copy link
Collaborator

@krtab krtab commented Jun 18, 2024

No description provided.

@zapashcanon
Copy link
Member

I'm going to bench this on Test-Comp first

@zapashcanon
Copy link
Member

Main: Nothing: 75 Reached: 613 Timeout: 527 Other: 1 Killed: 0
This PR: Nothing: 75 Reached: 609 Timeout: 531 Other: 1 Killed: 0

Also:

on      commonly  reached tasks, tool 1 took 1575.201145 sec. (mean 2.586537) and tool 2 took 1614.162636 sec. (mean 2.650513)

(tool1 is main, tool2 is this PR)

@zapashcanon
Copy link
Member

zapashcanon commented Jul 3, 2024

@krtab, what should we do with this one ?

(also could you rebase so I can restart the CI ?)

@zapashcanon
Copy link
Member

I rebased. Let's merge.

@zapashcanon zapashcanon merged commit 388f06d into OCamlPro:main Dec 4, 2024
4 checks passed
@krtab
Copy link
Collaborator Author

krtab commented Dec 4, 2024

Wasn't it worse? I wonder if we shouldn't have just ditch it.

@zapashcanon
Copy link
Member

The difference was quite small. I'm not sure it was significant... Do you believe the code to be better this way or should I revert?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants