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

0.21 Release #3674

Closed
davidhewitt opened this issue Dec 20, 2023 · 23 comments
Closed

0.21 Release #3674

davidhewitt opened this issue Dec 20, 2023 · 23 comments

Comments

@davidhewitt
Copy link
Member

Now that we've merged quite a lot of breaking improvements (😄) onto main, I think we should consider releasing before we go too much longer.

Given the recent soundness discussions, I think there's a few pieces that need resolving/merging first:

There's also a couple of pieces which are nearly there which I think would be very desirable to merge:

The main question, in my opinion, is what the gevent fix will look like. If it's not too heavy a performance impact, we could consider releasing once all the above is done.

If the gevent fix is a major bottleneck, we may want to also release the new Py2 API at the same time. This will create a significantly harder migration for users, however.

@adamreichold
Copy link
Member

If the gevent fix is a major bottleneck, we may want to also release the new Py2 API at the same time. This will create a significantly harder migration for users, however.

I thought about this as well, and I think realistically the better option for users is a version which has Py2 as an alternative API to facilitate migration even if unsound interactions with gevent are still present than rip off the band aid and fix both issues at once. The underlying reason is that I expect the contextvars fix to be so difficult to explain, not the least w.r.t. its performance profile, that fixing it by removing the pool really is the better option.

How to keep users safe regarding alpha Pythons?

I thought about this repeatedly and I am really uneasy about a version ceiling as this could really complicate maintenance of old software systems which should be completely usable, i.e. think of a researcher picking up a simulation model after it was left untouched for a few years.

What I could imagine doing is to require opt-in into building against alpha versions but I see much less of a case for doing so as using an alpha version does come with the expectation of breaking.

Due to the above, I would be glad if we could postpone making a decision on this and avoid blocking a 0.21 release on the issue.

@davidhewitt
Copy link
Member Author

Sure thing, I think it would be acceptable to make checks for #3555 land in a patch release anyway. I do feel quite strongly that we need some extra safety there, but let's discuss on that thread.

@davidhewitt
Copy link
Member Author

I thought about this as well, and I think realistically the better option for users is a version which has Py2 as an alternative API to facilitate migration even if unsound interactions with gevent are still present than rip off the band aid and fix both issues at once. The underlying reason is that I expect the contextvars fix to be so difficult to explain, not the least w.r.t. its performance profile, that fixing it by removing the pool really is the better option.

I'm definitely ok with moving ahead with that; if we can say that migrating to the Py2 API will improve performance, and fix soundness issues, that's a very compelling upgrade for users. Agreed that the contextvars fix seems pretty painful especially as it won't have a good abi3 solution, so it might be suitable just as a retrospective patch for 0.20 (and maybe older versions) depending on what we want to do about them.

I'll proceed to prep the next batch of PRs for the Py2 migration, in that case!

@adamreichold
Copy link
Member

Two things I wonder is, if we release 0.21 with Py2 as an alternative API

  • Must we change the name of Py2 to something lasting like PyXYZ avoid people having to rename everything again once we transition from Py2 to Py and Py to PyDetached?
  • Would downstream code be able to avoid the gevent issues by carefully avoiding any pool-based API? If so, we should probably document this as workaround to have PyO3-based extension at least be compatible with gevent even if still technically unsound.

@davidhewitt
Copy link
Member Author

Must we change the name of Py2 to something lasting like PyXYZ avoid people having to rename everything again once we transition from Py2 to Py and Py to PyDetached?

I would like us to release with a more permanent-sounding name than Py2. My favourite is still Py + PyDetached, but asking users to rename Py is even more breaking changes going into this release, so maybe we have to accept that ship has sailed and defer such a rename until the dust has settled? 🤔

TBH if the first step of the migration was to rename all items of Py to PyDetached, then it's not too bad. I wonder, we could potentially ship PyDetached as a type alias in an 0.20.1 patch release so that users can migrate that before the full bump to 0.21.0?

Would downstream code be able to avoid the gevent issues by carefully avoiding any pool-based API?

Yes, and I suggest we go further and gate off the pool-based API with an unsafe-backcompat feature which users can manually enable purely to help them migrate.

@adamreichold
Copy link
Member

Yes, and I suggest we go further and gate off the pool-based API with an unsafe-backcompat feature which users can manually enable purely to help them migrate.

To be honest, I think this is one of those path-dependent situations again. If we would start from scratch than Py + PyDetached is a clear winner. But since we can't, we should kill our darlings and bite the aesthetically unpleasing bullet of having to give the new and longer name to Py2.

I just don't see any material/technical reason to not give a permanent name like PyBound/PyLocked/PyAttached/PyBorrowed/PyPin/PyGuard to Py. We just have to make painful decision to settle on something. (Since making it short would really be nice here, I also wonder if we should drop the Py prefix? Of course, with the current design GIL/Gil<'py, T> would still make sense, but then again there is nogil and we might want to avoid emphasizing the GIL.)

Yes, and I suggest we go further and gate off the pool-based API with an unsafe-backcompat feature which users can manually enable purely to help them migrate.

Hhhmmm, not sold yet. How about a feature (or plain cfg flag, environment variable, etc.) to disable the pool-based API to be confident that it isn't used? I don't the think churn of everybody who is not yet willing to migrate have to turn this on is helpful. (Generally, I think it is better to not to try to force downstream projects into doing something by having them jump through hoops to keep doing what they are doing.)

@adamreichold
Copy link
Member

(Temp might be another candidate to emphasize their temporary nature compared to Py.)

@davidhewitt
Copy link
Member Author

Generally, I think it is better to not to try to force downstream projects into doing something by having them jump through hoops to keep doing what they are doing.

Fair enough. One option could just be to deprecate every API that touches the pool, and not bother with a feature? That way their code should keep compiling but just be quite noisy about that fact 😅

@davidhewitt
Copy link
Member Author

But since we can't, we should kill our darlings and bite the aesthetically unpleasing bullet of having to give the new and longer name to

I think you're right, and we can always revisit this again at, say, 1.0. Here's a brain dump of ideas of more names for Py2:

  • Single letter e.g. A for attached, H for handle (inspired by HPy), X because why not
  • PY in capitals, though seems too subtle maybe
  • Py! as a macro to hide a longer type name
  • all of the options you list above are also good

I'm reasonably sure I don't like Gil because I'd like to get us working with nogil as my next main task once we've got this API up.

I'll continue to ponder this evening while I do my son's bedtime for the next few hours...

@davidhewitt
Copy link
Member Author

I'm currently thinking Bound<'py, PyAny> might work? Or even maybe Bind<'py, PyAny>. It's relatively short, the 'py lifetime still hints at this being a python type even in Bound<'py, T> for some #[pyclass].

Another reasonable option is Ref<'py, PyAny>, which is quite short and the connection to "python reference" is not too big a leap. This is course conflicts with PyRef<T> a little, but it's probably easier to rename that later on than renaming Py<T> is. Plus, with a Sync and frozen default on #[pyclass] then PyRef<T> becomes even less relevant...

@adamreichold
Copy link
Member

I like Bound as it feels less overloaded than Ref especially since this is also the verb we are using in the docs of as_ref. I do feel that BoundBorrowed would need to be something else then. But RefBorrowed works even less. Maybe BoundRef?

@adamreichold
Copy link
Member

Or maybe even just Borrowed for Py2Borrowed in keeping with the plain Bound.

@davidhewitt
Copy link
Member Author

Agreed BoundBorrowed is a mouthful.

How would you feel about PyBorrowed (or just Borrowed)? BoundRef is quite nice too, though I do like using the "borrowed" terminology from the python C API. I'd be happy with any of those three, do you have a particular favourite?

@davidhewitt
Copy link
Member Author

Ah I see we had a cognitive race 😅

Let's try Bound and Borrowed then 👍

@alex
Copy link
Contributor

alex commented Jan 27, 2024

In #3113 we discussed raising the MSRV to 1.63, but ultimately decided to go to 1.56.

It's now been several months -- should we consider raising the MSRV to 1.63 for the 0.21 release? If so, I can easily put together a PR for that. Notably, this would allow us to drop the parking_lot dep.

@davidhewitt
Copy link
Member Author

While I love bumping MSRV, I think it would need to be 1.62 as per #3113 (comment)

I wonder though if it's better to keep it back for 0.22? It's not strictly necessary to bump right now, and there's so much changing in 0.21 anyway.

@alex
Copy link
Contributor

alex commented Jan 28, 2024

Ok, let's hold until after we get this large release out.

@alex
Copy link
Contributor

alex commented Jan 28, 2024

Note to future self: RHEL9 now has 1.71, they updated it in RHEL9.2 and RHEL9.3

@michaelgmiller1
Copy link

Any rough idea on timeline for this release? We're eager to upgrade to Pydantic v2, but can't until they've migrated to PyO3, which in turn requires this release!

@davidhewitt
Copy link
Member Author

migrated to PyO3

Pydantic are already using PyO3, but 0.21 will have a new API being implemented in #3684 that unlocks a lot of performance and memory wins, plus safety corrections. I suspect that this is what you've seen discussed in other threads.

I want to ship 0.21 ASAP as soon as #3684 is ready. @adamreichold and I have been chipping away at this for some months now and recently @Icxolu, @snuderl and @LilyFoote have been giving some much-appreciated additional momentum to the work. I feel like we're getting close now.

Definitely not this week.

Probably not the week after.

But at the rate we're going I'd really like us to ship this late Feb / early March.

@davidhewitt
Copy link
Member Author

I think once #3974 and #3975 are merged, I will prepare the PR for the final release!

@alex
Copy link
Contributor

alex commented Mar 20, 2024

its happening dot gif!!!!

@davidhewitt
Copy link
Member Author

And it has now happened, a week ago already 🚀

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

No branches or pull requests

4 participants