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

simplify traits and dyn-safety #23

Merged
merged 8 commits into from
Sep 4, 2023
Merged

simplify traits and dyn-safety #23

merged 8 commits into from
Sep 4, 2023

Conversation

Byron
Copy link
Owner

@Byron Byron commented Sep 4, 2023

Progress is now NestedProgress, RawProgress is now Progress, and there is
a new Count trait for solely counting things.

NobodyXu and others added 4 commits August 20, 2023 08:59
Fixed #21

`DynProgress` is sealed and implemented for any type that implements
`Progress`.

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
They are roughly half as fast due to the dynamic call overhead.
…em to be dyn-safe.

`Progress` is now `NestedProgress`, `RawProgress` is now `Progress`, and there is
a new `Count` trait for solely counting things.
@Byron
Copy link
Owner Author

Byron commented Sep 4, 2023

@NobodyXu After doing some refactoring I am in a place where I am happy to make a release. This release includes #22 which now provides a way to opt-in to dynamic hierarchical progress, which previously wasn't an option. In this version, for some reason, the performance is greatly improved and we are looking at only ~30% less performance for typical counting, and even that won't be an issue if counter() is used in places where high-frequency counting is expected.

Thus, in theory, it should be possible to replace NestedProgress with DynNestedProgress, maybe even automatically. I also believe that doing so will have no measurable performance impact, judging by the recent benchmark numbers.

With that in mind I will start reviewing the codebase soon and use dyn in plumbing crates by hand, leaving gix with gix_macros::momo as it should maximize convenience.

This should simplify downstream code and we just accept that we are dealing
with a threaded world.
This also comes with performance improvements as increments are now 250% faster.
@Byron Byron merged commit 5e21df7 into main Sep 4, 2023
2 checks passed
@Byron Byron deleted the simplify branch September 4, 2023 15:26
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.

2 participants