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

Potential optimizations #327

Open
5 of 8 tasks
benfdking opened this issue May 23, 2024 · 9 comments
Open
5 of 8 tasks

Potential optimizations #327

benfdking opened this issue May 23, 2024 · 9 comments
Labels
enhancement New feature or request rust Pull requests that update Rust code

Comments

@benfdking
Copy link
Collaborator

benfdking commented May 23, 2024

This is just a list of potential optimizations that we think could work.

Maintained list of optimizations

  • We currently use random numbers (uuid) for our caching system, my understanding is that generating random numbers could be expensive (at the very least it's 128-bit), could you
      1. move to 64-bit integers
      1. not use random. but a counter, so it is more deterministic and skips expensive random
  • Can we const fn more of the work upfront for building of dialects, (we could potentially use Perfect Hashes)
    • Could have an improvement on what is packaged
    • Could have an improvement impact on startup
  • Are there any bits where we still use String and .clone() for convenience?
  • Caching Depmath
  • Caching dependent type set
    • We know we do these lazily/optionally on call; is it worth introducing that branching?
  • A surprising amount of time is spent in DepthMap from_config. Could we "parse" the config into a super clean type first and then make the downstream "re-parsing" of every time from a slightly more flexible structure more efficient?
  • Will building with --features=jemalloc improve performance?

@gvozdvmozgu feel free to add any comments that you have thought of

@gvozdvmozgu
Copy link
Collaborator

It seems that creating a DepthMap is currently quite expensive and is not cached, meaning different rules generate their own DepthMap for the same segments.

@gvozdvmozgu
Copy link
Collaborator

Using const fn is unlikely at the moment. We need at least compile-time HashMaps. There is an attempt to implement this in this PR, but it requires unstable features, and I’m not sure how well it actually works. The PR appears to be abandoned.

@benfdking
Copy link
Collaborator Author

I was also looking at some of the flame graphs, and to me, there may be two potential areas of improvement:

  1. A descendant_type_set is a function where a lot of time is spent. Is there a way to speed it up descendant_type_set? We use AHashSet but since we are acting with very small sets, I wonder if just doing the comparison could be faster?
  2. A surprising amount of time is spent in DepthMap from_config, could we "parse" the config into a super clean type first and then make the downstream, "re-parsing" of every time from a slightly more flexible structure more efficient.

@gvozdvmozgu
Copy link
Collaborator

Will building with --features=jemalloc improve performance?

@gvozdvmozgu
Copy link
Collaborator

the descendant_type_set should ideally be cached, at least that's how it's done in the original implementation

@gvozdvmozgu
Copy link
Collaborator

We use .to_uppercase (which allocates a String) in some places for case-insensitive comparison. Instead, we could use something like UniCase<Cow<'static, str>>. It appears we already use UniCase in some parts of the code. And accordingly, change the signature of Matchable::simple to Option<(AHashSet<UniCase<Cow<'static, str>>, AHashSet<&'static str>)> for example.

@benfdking
Copy link
Collaborator Author

Just thought I'd upload a latest flamegraph.

flamegraph

@benfdking benfdking added enhancement New feature or request rust Pull requests that update Rust code labels Aug 8, 2024
@Victorthedev
Copy link

Can i work on one of the remaining 3 tasks?

@benfdking
Copy link
Collaborator Author

benfdking commented Sep 23, 2024

Hey @Victorthedev, feel free to look at anything you think is valuable. Happy to support any contributions!

There's still lots to do but this may be slightly out of date. Some of the ideas are still right but I would definitely try recreating that flamegraph and seeing where the opportunities lie.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request rust Pull requests that update Rust code
Projects
None yet
Development

No branches or pull requests

3 participants