crossbeam-rs / crossbeam

Tools for concurrent programming in Rust
Apache License 2.0
7.39k stars 467 forks source link

Move CachePadded to separate sub-crate? #1066

Open mgeier opened 9 months ago

mgeier commented 9 months ago

I have been using https://github.com/smol-rs/cache-padded/ as a dependency until it has been deprecated (https://github.com/smol-rs/cache-padded/pull/11) and then, as suggested, I switched to crossbeam_utils::CachePadded.

I didn't really like that this forced an unneeded dependency (cfg-if, mentioned at https://github.com/smol-rs/cache-padded/issues/10#issuecomment-1237286495) onto my crate.

I kinda accepted this, but recently, I found another inconvenience: it also forces me to bump the MSRV of my crate due to #1037, which is AFAICT unrelated to CachePadded.

Would it be possible to move CachePadded to a separate sub-crate to avoid those problems?

Alternatively, both problems might be avoidable by moving parts of crossbeam_utils to separate "features" (which I could deactivate for my crate), but I don't know if that's feasible?

taiki-e commented 9 months ago

I didn't really like that this forced an unneeded dependency (cfg-if, mentioned at smol-rs/cache-padded#10 (comment)) onto my crate.

Well, as for cfg-if I don't really like it and will eventually remove it. Work on revisiting dependencies is in progress, and a few dependencies were recently removed from some sub-crates (https://github.com/crossbeam-rs/crossbeam/pull/1058, https://github.com/crossbeam-rs/crossbeam/pull/1045).

I kinda accepted this, but recently, I found another inconvenience: it also forces me to bump the MSRV of my crate due to #1037, which is AFAICT unrelated to CachePadded.

Do you have any concrete requirements? I think we can consider relaxing MSRV if it is really needed. For example, https://github.com/crossbeam-rs/crossbeam/pull/1056 has relaxed the MSRV a bit that https://github.com/crossbeam-rs/crossbeam/pull/1037 bumped.

Would it be possible to move CachePadded to a separate sub-crate to avoid those problems?

Alternatively, both problems might be avoidable by moving parts of crossbeam_utils to separate "features" (which I could deactivate for my crate), but I don't know if that's feasible?

One of the reasons cache-padded was deprecated was the maintenance burden, such as reflecting updates in crossbeam-utils. If we move the cache-padded to crossbeam repo and making the implementation a symbolic link to the crossbeam-utils code, it would be possible to reduce the maintenance burden. That said, I'm not sure it is worth having a separate crate for it, as said in https://github.com/smol-rs/cache-padded/issues/10#issuecomment-1237286495. (I'm not interested in adding new dependencies to crossbeam-utils.)

mgeier commented 9 months ago

Well, as for cfg-if I don't really like it and will eventually remove it.

That's great news!

Do you have any concrete requirements?

No. I just dislike bumping the MSRV of my crate without any real reason. Every bump is a potential obstacle for users of my crate and of dependent crates. And even if the current MSRV is not a problem, there might be future bumps in crossbeam-utils that would not be necessary for CachePadded.

I'm not against bumping the MSRV in general, I just would like to avoid doing it gratuitously.

If we move the cache-padded to crossbeam repo and making the implementation a symbolic link to the crossbeam-utils code, it would be possible to reduce the maintenance burden.

With an additional sub-crate there would of course be an added maintenance cost when updating crate versions, but I would hope that it is small enough to still be feasible.

I'm not interested in adding new dependencies to crossbeam-utils.

Yeah, I understand, but this would only be an "internal" dependency to a sub-crate.

To make my suggestion a bit more concrete, I've crated a draft PR: #1069.

There are a few details which would have to be ironed out, but it can hopefully serve as a basis for discussion.

What do you think about it?

taiki-e commented 9 months ago

Well, as for cfg-if I don't really like it and will eventually remove it.

That's great news!

Filed https://github.com/crossbeam-rs/crossbeam/pull/1072 for this.

Do you have any concrete requirements?

No. I just dislike bumping the MSRV of my crate without any real reason. Every bump is a potential obstacle for users of my crate and of dependent crates.

The MSRV was not bumped for no reason. It was just bumped for APIs you are not using (https://github.com/crossbeam-rs/crossbeam/issues/1033).

I'm not interested in adding new dependencies to crossbeam-utils.

Yeah, I understand, but this would only be an "internal" dependency to a sub-crate.

To make my suggestion a bit more concrete, I've crated a draft PR: #1069.

I said "new dependencies", not "new public dependencies" or "new external dependencies". "new 'internal' dependency" is also a "new dependency". Your PR seems to be doing exactly what I thought I didn't want to do when I wrote that comment.

(Also, your comments make me feel that you are saying, "I don't like the fact that my crate's dependency has a dependency, but I would like you to add a dependency to your crate".)

taiki-e commented 9 months ago

Well, as for cfg-if I don't really like it and will eventually remove it.

That's great news!

Filed #1072 for this.

This has been released in crossbeam-utils v0.8.19.

mgeier commented 9 months ago

Thanks for removing cfg-if, that's great!

The MSRV was not bumped for no reason. It was just bumped for APIs you are not using (#1033).

Sure, you didn't just bump it for the fun of it. But when I bump the MSRV of my crate, there will be no real reason.

I said "new dependencies", not "new public dependencies" or "new external dependencies". "new 'internal' dependency" is also a "new dependency". Your PR seems to be doing exactly what I thought I didn't want to do when I wrote that comment.

Yeah, I assumed that "internal" dependencies are fine, but I guess they also cause some overhead when downloading crates.

I have created an alternative approach (based on your comment about symbolic links above): #1075. Would that be more reasonable?

(Also, your comments make me feel that you are saying, "I don't like the fact that my crate's dependency has a dependency, but I would like you to add a dependency to your crate".)

Yeah, I didn't intend it to sound like this. I assumed (wrongly) that "internal" dependencies don't count.