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

Remove need for tuple_for_each!, tuple_len! and simplify gen_conditions! a lot #99

Merged

Conversation

poliorcetics
Copy link
Contributor

@poliorcetics poliorcetics commented Nov 19, 2022

By using named fields instead of anonymous one in the $mod_name::Streams structs, we can completely
remove the need for the very tuple_for_each! macros, which makes the code more maintainable and
the crate faster to compile (though that wasn't a limitation at this point).

I also took the opportunity to remove the tuple_len! macro because recursivity is heavy on the compiler. The generated code for this part doesn't slow down at all because it's all const-computed anyway (but instead of producing an imbalanced binary tree, we produce a more "normal" one which is much easier to deal with for rustc).

Finally, I greatly simplified the gen_conditions! macro by removing the recursion by using the Indexes enum again (that trick is awesome, I'm glad I finally found a crate to share it with 😄), which (again) removes a very unbalanced tree of 0 + 1 + 1 + 1 + ... in the generated code.

This notably makes life easier for contributors who have clippy configured by default
By using named fields instead of anonymous one in the `$mod_name::Streams` structs, we can completely
remove the need for the very `tuple_for_each!` macros, which makes the code more maintainable and
the crate faster to compile (though that wasn't a limitation at this point).
@poliorcetics
Copy link
Contributor Author

For me it's a 20% improvement, probably because the generated code is much more optimiser-friendly because I don't think I did that much

critcmp main-branch remove-macro-branch

group              main-branch                            remove-macro-branch
-----              -----------                            -------------------
tuple::merge 10    1.21   1196.7±7.90ns        ? ?/sec    1.00    991.3±8.45ns        ? ?/sec

@poliorcetics poliorcetics changed the title Remove need for tuple_for_each! Remove need for tuple_for_each!, tuple_len! and simplify gen_conditions! a lot Nov 19, 2022
@matheus-consoli
Copy link
Collaborator

that's really awesome! thank you!

@matheus-consoli matheus-consoli merged commit e3f61cd into yoshuawuyts:main Nov 19, 2022
@poliorcetics poliorcetics deleted the remove-need-for-tuple-for-each branch November 19, 2022 18:21
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.

None yet

2 participants