Consider switching from pin-project to pin-project-lite

[jplatte]

Feature Request

Crates

tracing-futures

Motivation

pin-project-lite, in contrast to pin-project, does not depend on syn and quote and as such is much lighter on compile times. futures-rs switched recently, see that PR for compile time differences.

Proposal

Switch from pin-project to pin-project-lite.

Alternatives

Do nothing. Other things (hyper, async-tungestenite, ...) are still pulling in pin-project so it will rarely make a difference to end users (now at least).

[davidbarsky]

Sorry for the delay in responding. I'm somewhat inclined to avoid making this switch, as I think we'll be taking a dependency on syn and quote anyways to rewrite tracing's macros. Any gain that might come from this change risks being erased. What do you think?

[jplatte]

In the benchmark from the PR I linked to, pin-project-internal + pin-project take 1.42s to compile independently from syn & quote. pin-project-lite takes 0.2s to compile in the same benchmark. Considering how little tracing is using pin-project / how easy it was to switch, this seems desireable even without being able to remove syn & quote from the dependencies.

[davidbarsky]

In the benchmark from the PR I linked to, pin-project-internal + pin-project take 1.42s to compile independently from syn & quote. pin-project-lite takes 0.2s to compile in the same benchmark.

My bad, I didn't look closely enough. That's a compelling argument in favor of this change, then.

[taiki-e]

In the benchmark from the PR I linked to, pin-project-internal + pin-project take 1.42s to compile independently from syn & quote. pin-project-lite takes 0.2s to compile in the same benchmark. Considering how little tracing is using pin-project / how easy it was to switch, this seems desireable even without being able to remove syn & quote from the dependencies.

I disagree with this. "It takes N seconds independently" is not always a problem (parallel compile handles it well). See this comment that linked in the PR you linked to.

• As mentioned in pin-project-lite's readme, if you already have proc-macro-related dependencies in your crate's dependency graph, adding pin-project to a dependency has little impact on compile time.
  I have previously investigated how little the effect is actually. See tokio-rs/tokio#1588 (comment) for that.

[jplatte]

You're right but with tokio already using pin-project-lite and futures-rs having switched recently, it's almost impossible to not depend on pin-project-lite in an app that uses async/.await. I'd prefer if there wasn't multiple dependencies that do the same thing in my dependency graph, that's why I'm sending these patches.

Further, while parallelization might mean that using pin-project instead of pin-project-lite in tracing doesn't improve compile times for tracing itself, it may improve compile times for dependent crates with a larger dependency graph, such that parallelization is already being used maximally for a large part of the build process. And there's also the issue of power consumption, and CPU throttling that can worsen compile times even if additional work is done fully on an otherwise unused core.

All-in-all I don't see why tracing should not switch, considering it's so simple (PR is up at #1185, diffstat +52 −33).

[taiki-e]

Further, while parallelization might mean that using pin-project instead of pin-project-lite in tracing doesn't improve compile times for tracing itself, it may improve compile times for dependent crates with a larger dependency graph, such that parallelization is already being used maximally for a large part of the build process. And there's also the issue of power consumption, and CPU throttling that can worsen compile times even if additional work is done fully on an otherwise unused core.

That's true, but it's basically only if consider the compile time in a single clean build. (And its impact should actually be small enough.)
If you actually run into pin-project-lite's limitations/confusing error messages and debug them/search workaround, it's very easy to consume more time and energy.

FYI, I already explained this in the comment linked in the previous comment:

The error messages emitted by the current pin-project-lite are very poor and may require some knowledge of macro or pin API to determine if it is a macro limitation or a legitimate error.
In fact, tokio and async-std had unsoundness that seemed to be due to a misunderstanding of this: tokio-rs/tokio#2612, async-rs/async-std#903
(They did unsafe pin-projections for something that the pin-project-lite didn't seem support, but the error is correct and manual pin-projections were unsound)
pin-project has no limitation like pin-project-lite and can emit decent errors in most cases.

---

I'd prefer if there wasn't multiple dependencies that do the same thing in my dependency graph, that's why I'm sending these patches.

In the same comment, I said my opinion on this:

pin-project-lite is very lightweight, and compile-time does not change whether both pin-project and pin-project-lite are included in the dependency or only pin-project is included in the dependency.

---

All-in-all I don't see why tracing should not switch, considering it's so simple (PR is up at #1185, diffstat +52 −33).

To be honest, I'm not strongly against doing this in tracing. However, I can easily imagine someone will claim the same with a link to this issue for other projects that in similar situations.
Therefore, I felt it was necessary to explain the costs that could be incurred in exchange for a "slight reduction in compile time" that was doubtful of its actual effect.

[jplatte]

Okay, I see where you're coming from. Let's see what the maintainers think. I still think even though it's just fresh compiles this still makes somewhat of a difference when frequently switching between toolchains, frequently updating due to being on Nightly, and for CI. But you're right that in most circumstances it's not so much of a problem.

Also with you having written and maintaining both crates, there is no additional supply chain attack vector when using both – if anything, I would consider pin-project-lite to be harder to review due to being macro_rules! code, and thus less trustworthy 😄.

[taiki-e]

if anything, I would consider pin-project-lite to be harder to review due to being macro_rules! code, and thus less trustworthy 😄.

Yeah, pin-project is definitely more robust than pin-project-lite. (Especially in parsing, pin-project-lite needs to do everything itself, and actually, most of pin-project-lite's known limitations/bugs are about parsing. By the way, for the code generation, there are tests to track changes in the generated code in both crates, so basically there is no problem.)