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

Polish before alpha #21

Merged
merged 34 commits into from
Aug 14, 2024
Merged

Polish before alpha #21

merged 34 commits into from
Aug 14, 2024

Conversation

skeet70
Copy link
Member

@skeet70 skeet70 commented Jul 24, 2024

No description provided.

@skeet70 skeet70 requested a review from a team as a code owner July 24, 2024 19:38
@skeet70 skeet70 requested review from giarc3 and coltfred and removed request for a team and giarc3 July 24, 2024 19:38
Copy link

github-actions bot commented Jul 31, 2024

File Coverage Lines
All files 77% 77%
src/gen_java/mod.rs 88% 88%
src/lib.rs 54% 54%

Minimum allowed coverage is 0%

Generated by 🐒 cobertura-action against af6eb00

@skeet70 skeet70 force-pushed the polish-before-alpha branch from c7104a3 to 0881859 Compare August 5, 2024 17:17
skeet70 added 13 commits August 5, 2024 11:35
What if the logic is fine but the Java FFI is slow, so the delay
finishes before the call from Rust gets back for it to await the
abortable?
and wait longer than the times we're seeing in CI on the delay. The free
is still being called, so nothing looks like it's leaking, but it is way
slower than expected vs local (or even passing CI runs, which were
faster than local)
Try extending the delay a bunch
skeet70 added 6 commits August 6, 2024 18:58
Right now when it works in CI the free happens ~1.3ms after the call to
the delay trait (which makes sense given the 1ms delay in Rust before it
aborts). When it doesn't work though the free doesn't happen until ~.6ms
after the delay call fully finishes, so 100.5ms after the call to the
delay trait. It seems like a bug where the FreeImpl isn't being called
from Rust in time, but I want to see if bumping the delay to 200ms
results in 1.3/200.5, or 1.3/100.5 still
We can't trust Java's `CompletableFuture` to wake a suspended thread correctly when the
`CompletableFuture` being blocked on is `complete()`, so we're manually
busy waiting the futures thread for now.
@skeet70 skeet70 force-pushed the polish-before-alpha branch from 382ab7c to 5f7f2de Compare August 12, 2024 18:28
Copy link
Member

@giarc3 giarc3 left a comment

Choose a reason for hiding this comment

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

LGTM

@skeet70 skeet70 merged commit c865946 into main Aug 14, 2024
7 checks passed
@skeet70 skeet70 deleted the polish-before-alpha branch August 14, 2024 21:58
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