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

Inline Text.Fuzzy to add INLINABLE pragmas #2215

Merged
merged 4 commits into from Sep 19, 2021
Merged

Inline Text.Fuzzy to add INLINABLE pragmas #2215

merged 4 commits into from Sep 19, 2021

Conversation

pepeiborra
Copy link
Collaborator

@pepeiborra pepeiborra commented Sep 19, 2021

I found a noticeable perf regression in completions performance when trying to upgrade our custom Sigma IDE from HLS 1.2 to 1.4. The reason is that 1.3 added support for local completions from hiedb and that brings an additional >500k identifiers into scope, so the performance of fuzzy matching becomes a bottleneck.

The fuzzy package is parametric on the text type via the TextualMonoid type class. Specializing the Fuzzy.filter function to Text cuts the allocations by 66% and the time by ~33%.

I sent a PR upstream, but it might take a while to get merged. Since the license (MIT) allows for it, let's inline the code here and drop the dependency until then.

Copy link
Member

@jneira jneira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with the temporary inclusion, with the already included appropiate comments

@pepeiborra pepeiborra added the merge me Label to trigger pull request merge label Sep 19, 2021
@mergify mergify bot merged commit 2869077 into master Sep 19, 2021
pepeiborra added a commit that referenced this pull request Sep 20, 2021
pepeiborra added a commit that referenced this pull request Sep 20, 2021
mergify bot added a commit that referenced this pull request Sep 21, 2021
* Revert "Inline Text.Fuzzy to add INLINABLE pragmas (#2215)"

This reverts commit 2869077.

* do not revert ghcide version bump

* Update version of fuzzy in stack.yaml

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
pepeiborra added a commit that referenced this pull request Sep 21, 2021
pepeiborra added a commit that referenced this pull request Sep 21, 2021
cdsmith pushed a commit to cdsmith/haskell-language-server that referenced this pull request Sep 21, 2021
* Inline Text.Fuzzy to add INLINABLE pragmas

* add note

* fixup fuzzy

* bump ghcide version number
cdsmith pushed a commit to cdsmith/haskell-language-server that referenced this pull request Sep 21, 2021
* Revert "Inline Text.Fuzzy to add INLINABLE pragmas (haskell#2215)"

This reverts commit 2869077.

* do not revert ghcide version bump

* Update version of fuzzy in stack.yaml

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
pepeiborra added a commit that referenced this pull request Sep 22, 2021
* Revert "Inline Text.Fuzzy to add INLINABLE pragmas (#2215)"

This reverts commit 2869077.

* Fuzz in parallel

* Efficiently with vectors

* use mapMaybe for compat. with older versions

* switch to stable sort

* clean ups
pepeiborra added a commit that referenced this pull request Sep 23, 2021
* Revert "Inline Text.Fuzzy to add INLINABLE pragmas (#2215)"

This reverts commit 2869077.

* Fuzz in parallel

* Efficiently with vectors

* use mapMaybe for compat. with older versions

* switch to stable sort

* clean ups
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants