taiki-e / pin-project-lite

A lightweight version of pin-project written with declarative macros.
https://docs.rs/pin-project-lite
Apache License 2.0
216 stars 15 forks source link

Break up `__pin_project_internal` #71

Closed nnethercote closed 2 years ago

nnethercote commented 2 years ago

I have been investigating expensive declarative macros recently. pin_project_lite::pin_project showed up as expensive for the futures-lite crate. This macro is very large, with 47 rules, many of these being internal rules. I thought that splitting it up into lots of smaller macros might help with performance. It only made a small (2%) performance improvement, but I think it increases readability enough that it's worth doing anyway. So I made this pull request.

nnethercote commented 2 years ago

It improves performance because it avoids lots of rule matching attempts that cannot succeed.

nnethercote commented 2 years ago

I just upgraded to the latest Nightly and ran cargo +nightly --all --all-features. For the ui tests I got lots of messages about expected output vs actual output, but no test failures. Those were just marked as wip... "work in progress"??

That command also changed lots of .stderr files, which was unexpected. A lot of the changes are due to the macro name changes, e.g.:

-  = note: this error originates in the macro `$crate::__pin_project_internal` (in Nightly builds, run with -Z macro-backtrace for more info)
+  = note: this error originates in the macro `$crate::__pin_project_make_drop_impl` (in Nightly builds, run with -Z macro-backtrace for more info)

but there are also some like this, which I don't understand:

-error: no rules expected the token `[`
- --> tests/ui/pin_project/invalid-bounds.rs:3:1
+error: no rules expected the token `:`
+ --> tests/ui/pin_project/invalid-bounds.rs:4:33
   |
-3 | / pin_project! {
-4 | |     struct Generics1<T: 'static : Sized> { //~ ERROR no rules expected the token `:`
-5 | |         field: T,
-6 | |     }
-7 | | }
-  | |_^ no rules expected this token in macro call
-  |
-  = note: this error originates in the macro `$crate::__pin_project_internal` (in Nightly builds, run with -Z macro-backtrace for more info)
+4 |     struct Generics1<T: 'static : Sized> { //~ ERROR no rules expected the token `:`
+  |                                 ^ no rules expected this token in macro call
taiki-e commented 2 years ago

For the ui tests I got lots of messages about expected output vs actual output, but no test failures.

Locally, ui test output is blessed by default.

but there are also some like this, which I don't understand:

The new diagnosis seems correct. I guess the rule reordering or rule separation may have changed the tokens that are considered expected to be matched first.

bors[bot] commented 2 years ago

Build succeeded:

nnethercote commented 2 years ago

Thanks!

taiki-e commented 2 years ago

Published in 0.2.9.