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

Eliminate most usages of asynchronous processing #657

Merged
merged 3 commits into from
Dec 8, 2021
Merged

Conversation

timholy
Copy link
Owner

@timholy timholy commented Dec 3, 2021

The locking of require_lock changes the order in which package callbacks run when two things come together:

  • the callbacks are run asynchronously
  • they call Base methods that acquire require_lock

This eliminates the asynchronous processing, at the cost of about 200ms extra "visible" latency when loading ImageCore. The increased latency is only a consequence of the second commit; it would be better to drop it, in which case it would be necessary to add a yield() after

@eval using TrackRequires
and
@eval using TrackRequires2
which would simulate returning to the REPL under normal usage. However, running these synchronously does have advantages, so it may be better to try to regain the lost performance through other means.

@timholy timholy changed the title Add delays in Requires tests Eliminate most usages of asynchronous processing Dec 5, 2021
On nightly, the Revise tests hang when code_coverage=true.
This seems to be a consequence of new locking in the `Base.require`
pipeline while executing package operations in a different task,
for which attempting to take the lock from a different task fails.
Similar to the previous commit, `watch_package` also executes
operations that are now protected by `Base.require_lock`.
Making this synchronous eliminates the risk of changing task
order.
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.

1 participant