serialize: base64: allow LF in addition to CRLF and optimize slightly #19594
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Arcterus
added a commit
to Arcterus/coreutils
that referenced
this pull request
Dec 6, 2014
Arcterus
added a commit
to Arcterus/coreutils
that referenced
this pull request
Dec 6, 2014
| @@ -14,6 +14,7 @@ | |||
|
|
|||
| pub use self::FromBase64Error::*; | |||
| pub use self::CharacterSet::*; | |||
| pub use self::Newline::*; | |||
There was a problem hiding this comment.
Could you leave this out? The other two reexports were just added to make the initial namespacing transition easier, but we don't want to keep them around forever.
|
What do the benchmark numbers look like before and after this? |
|
I have only benchmarked this using GNU Setup for benchmarking: Original: New: New (using LF instead of CRLF, so we don't have to replace CRLF with LF ourselves): |
|
r? |
|
needs a rebase |
It is useful to have configurable newlines in base64 as the standard leaves that for the implementation to decide. GNU `base64` apparently uses LF, which meant in `uutils` we had to manually convert the CRLF to LF. This made the program very slow for large inputs. [breaking-change]
|
Rebased. |
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Dec 9, 2014
It is useful to have configurable newlines in base64 as the standard leaves that for the implementation to decide. GNU `base64` apparently uses LF, which meant in `uutils` we had to manually convert the CRLF to LF. This made the program very slow for large inputs. [breaking-change]
cnd
pushed a commit
to uutils/coreutils
that referenced
this pull request
Dec 10, 2014
base64: fix build (assuming rust-lang/rust#19594 is merged)
jbcrail
pushed a commit
to jbcrail/coreutils
that referenced
this pull request
Apr 29, 2015
jbcrail
pushed a commit
to jbcrail/coreutils
that referenced
this pull request
Apr 29, 2015
base64: fix build (assuming rust-lang/rust#19594 is merged)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It is useful to have configurable newlines in base64 as the standard
leaves that for the implementation to decide. GNU
base64apparentlyuses LF, which meant in
uutilswe had to manually convert the CRLF toLF. This made the program very slow for large inputs.
[breaking-change]