Skip to content

Use AnyIO with fast_acquire synchronization #953

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 3 commits into from
Mar 27, 2025

Conversation

MarkusSintonen
Copy link
Contributor

Summary

Related to agronholm/anyio#761 and #922 (comment)

This uses AnyIOs fast_acquire-mode in its synchronization primitives (instead of replacing AnyIO with vanilla asyncio). Which gives a big performance boost (see above comment lint).

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

@graingert
Copy link
Member

nice this looks good!

This feature was added to anyio specifically for httpcore, so I'm going to merge it even though I'm the only review

@graingert graingert merged commit 973cbdd into encode:master Mar 27, 2025
5 checks passed
@tomchristie
Copy link
Member

@graingert No. Please don't do that on this PR.

This was my latest comment regarding this... #922 (comment)

Here's a comment that'd make me wary of jumping onto fast_acquire=True... agronholm/anyio#761 (review)

tomchristie added a commit that referenced this pull request Mar 27, 2025
@graingert
Copy link
Member

Happy for a revert, does #927 remove a lock on the async code path and therefore make this PR redundant?

@tomchristie
Copy link
Member

I'm not talking about #927 or directing my attention there at the moment.

@tomchristie
Copy link
Member

Revert PR at #1002.

@Nivyaal-zenity
Copy link

When will the tag related to this change here and on httpx will be relaesed?

tomchristie added a commit that referenced this pull request Mar 31, 2025
@ofek
Copy link

ofek commented Apr 19, 2025

I'm confused about what happened here; what was the technical issue?

@zanieb
Copy link
Contributor

zanieb commented Apr 19, 2025

@ofek
Copy link

ofek commented Apr 19, 2025

Ah I missed the second comment, thanks! I don't do enough async in Python to know what that means but I'll take a deeper dive eventually because such performance improvements always intrigue me.

@agronholm
Copy link
Contributor

agronholm commented Apr 26, 2025

@graingert No. Please don't do that on this PR.

This was my latest comment regarding this... #922 (comment)

Here's a comment that'd make me wary of jumping onto fast_acquire=True... agronholm/anyio#761 (review)

Can you explain this to me, as it didn't make a lick of sense to me? Was this about fast_acquire=True skipping the checkpoint? asyncio.Lock never had that checkpoint to begin with, so what do you suppose is gained by reverting the commit?

@tomchristie
Copy link
Member

what do you suppose is gained by reverting the commit?

@agronholm So. The priority at the time was bumping a release from 1.0.7 to 1.0.8 in order to resolve #1005.

Mixing in other concerns introduces risk, rather than just prioritising the necessary fix for Python 3.14.0a6.


Also I've been doing some background work on a significantly simplified implementation. I've got a connection pool approach that's smaller, more readable, and easier to reason about. It also happens to result in faster & more stable response characteristics under load. (significant improvement vs. current httpx, marginal improvement vs. aiohttpx)

I'm not ready to share the work publicly yet, tho will update shortly.

@agronholm
Copy link
Contributor

what do you suppose is gained by reverting the commit?

@agronholm So. The priority at the time was bumping a release from 1.0.7 to 1.0.8 in order to resolve #1005.

Mixing in other concerns introduces risk, rather than just prioritising the necessary fix for Python 3.14.0a6.

Also I've been doing some background work on a significantly simplified implementation. I've got a connection pool approach that's smaller, more readable, and easier to reason about. It also happens to result in faster & more stable response characteristics under load. (significant improvement vs. current httpx, marginal improvement vs. aiohttpx)

I'm not ready to share the work publicly yet, tho will update shortly.

OK, thanks for the quick response. I was previously left with the impression that you had reservations about fast_acquire, but perhaps I misunderstood.

@tomchristie
Copy link
Member

I was previously left with the impression that you had reservations about fast_acquire, but perhaps I misunderstood.

I don't really know enough about fast_acquire to take an informed stance on that.

(Incidentally, thanks so much for your work on anyio, and helping bring structured concurrency patterns to Python's asyncio.)

@agronholm
Copy link
Contributor

Yeah, so the summary is that with fast_acquire=True, AnyIO locks work pretty much the same way as asyncio locks (hence the performance parity). It does that by skiping a checkpoint, which on Trio is considered faux pas where everything with async with, async for or await should go through a checkpoint, but on asyncio there are no such considerations. Basically asyncio cuts corners and is faster because of that. fast_acquire=True just cuts corners the same way asyncio.Lock does.

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.

7 participants