Skip to content

Remove isAlphaNum in LBS decodeLenient #21

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

Merged
merged 2 commits into from
Dec 5, 2018
Merged

Remove isAlphaNum in LBS decodeLenient #21

merged 2 commits into from
Dec 5, 2018

Conversation

ocheron
Copy link
Contributor

@ocheron ocheron commented Apr 7, 2017

Using isAlphaNum makes decodeLenient much slower that it should be.
Rechunking also must be incorrect if non-ASCII characters are accepted instead of being filtered.
This PR replaces the call by simpler ASCII-only functions.

Before:

benchmarking lazy/large/normal/decode
time                 367.4 μs   (366.8 μs .. 368.0 μs)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 367.3 μs   (366.7 μs .. 367.8 μs)
std dev              1.810 μs   (1.486 μs .. 2.328 μs)

benchmarking lazy/large/normal/decodeLenient
time                 6.071 ms   (6.055 ms .. 6.084 ms)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 6.063 ms   (6.055 ms .. 6.071 ms)
std dev              24.26 μs   (19.38 μs .. 31.04 μs)

After:

benchmarking lazy/large/normal/decode
time                 367.3 μs   (366.8 μs .. 367.9 μs)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 367.8 μs   (367.2 μs .. 368.4 μs)
std dev              1.921 μs   (1.597 μs .. 2.595 μs)

benchmarking lazy/large/normal/decodeLenient
time                 1.516 ms   (1.514 ms .. 1.518 ms)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 1.517 ms   (1.516 ms .. 1.519 ms)
std dev              5.706 μs   (4.445 μs .. 7.802 μs)

This call is slow because it tests and accepts Unicode characters
that don't make much sense for base64.
@23Skidoo 23Skidoo merged commit bebeb27 into haskell:master Dec 5, 2018
@23Skidoo
Copy link
Member

23Skidoo commented Dec 5, 2018

Merged, thanks!

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