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

Fix bug #25 for GHC 9.4 and text-2 #26

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

skogsbaer
Copy link
Contributor

No description provided.

@skogsbaer
Copy link
Contributor Author

@sternenseemann @tvh we should merge this PR. Who has the rights to do so?

Copy link
Contributor

@tvh tvh left a comment

Choose a reason for hiding this comment

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

@skogsbaer We depend on the hashes being stable. This would therefore be a breaking change. Do you see a way to still use UTF-16 in the new version? Maybe via Data.Text.Lazy?

@skogsbaer
Copy link
Contributor Author

Lazy text also uses UTF-8 internally (it's just a list of strict tests). I don't see a way to retain backwards compatibility without introducing a performance overhead, e.g. by converting UTF-8 to UTF-16 to do the hashing.

@tvh
Copy link
Contributor

tvh commented May 27, 2023

I agree there is no way to avoid that overhead. The reason why I mentioned lazy text is that it would avoid having to have all in memory at the same time.

sternenseemann added a commit to NixOS/nixpkgs that referenced this pull request Jul 3, 2023
A potential fix for the problem is still in discussion: factisresearch/large-hashable#26
rvl pushed a commit to rvl/nixpkgs that referenced this pull request Jul 19, 2023
rvl pushed a commit to rvl/nixpkgs that referenced this pull request Jul 20, 2023
rvl pushed a commit to rvl/nixpkgs that referenced this pull request Jul 25, 2023
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

3 participants