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

Pure-haskell "memchr" #555

Merged
merged 5 commits into from
Feb 1, 2024
Merged

Pure-haskell "memchr" #555

merged 5 commits into from
Feb 1, 2024

Conversation

chreekat
Copy link
Contributor

This removes use of utils.c under -fpure-haskell.

(Technically there is still one use site of _hs_text_memcmp2, but it's guarded by MIN_VERSION_BASE(4,11,0). Thus -fpure-haskell is not compatible with ghc-8.2. I think I'm ok with that.)

This PR will conflict with #545 , so I'll keep this one a draft until that one is finalized. Plus, my use of toList is probably dumb, and I'd like to come up with a better way of doing it.

@chreekat
Copy link
Contributor Author

I just noticed that all callsites are of form cSizeToInt . memchr, so I'll probably move that into the definition, just as I did with the arguments.

@chreekat
Copy link
Contributor Author

chreekat commented Jan 27, 2024

What would be some good benchmarks to run? I reached blindly and grabbed these ones:

./dist-pure/build/x86_64-linux/ghc-9.2.8/text-2.1/b/text-benchmarks/build/text-benchmarks/text-benchmarks --baseline fast.baseline -p '$0 ~ /Sort/ || $0 ~ /Indices/ || $0 ~ /Lines/'
All
  ReadLines
    Text:                 OK (0.32s)
      44.8 ms ± 4.5 ms,       same as baseline
  FileIndices
    Text:                 OK (0.25s)
      85.2 ms ± 2.7 ms, 6968% more than baseline
    LazyText:             OK (0.27s)
      2.22 ms ± 123 μs,       same as baseline
  Programs
    Sort
      Text:               OK (0.80s)
        268  ms ± 6.7 ms, 87% more than baseline
      LazyText:           OK (0.89s)
        296  ms ±  13 ms, 58% more than baseline
      TextByteString:     OK (0.59s)
        195  ms ± 5.8 ms, 344% more than baseline
      LazyTextByteString: OK (0.56s)
        186  ms ±  14 ms, 258% more than baseline
      TextBuilder:        OK (0.57s)
        190  ms ± 5.6 ms, 318% more than baseline

(This is comparison of -fpure-haskell versus -fno-pure-haskell)

@Bodigrim
Copy link
Contributor

I'm not too worried about performance in -fpure-haskell mode, so overall looks sane to me as is.

@chreekat chreekat marked this pull request as ready for review January 31, 2024 07:15
@chreekat
Copy link
Contributor Author

This is ready now.

@Bodigrim Bodigrim requested a review from Lysxia February 1, 2024 00:13
@Bodigrim Bodigrim merged commit 456783a into haskell:master Feb 1, 2024
27 checks passed
@Bodigrim
Copy link
Contributor

Bodigrim commented Feb 1, 2024

Yay! Thanks @chreekat!

@chreekat
Copy link
Contributor Author

chreekat commented Feb 5, 2024

Awesome! Now I can go back to converting my console app to a web app 😂

@chreekat chreekat deleted the b/pure-utils branch February 5, 2024 06:02
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.

3 participants