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

Polish and documentation work for fastparse.byte #125

Merged
merged 3 commits into from Sep 8, 2016
Merged

Conversation

lihaoyi
Copy link
Member

@lihaoyi lihaoyi commented Sep 4, 2016

This is a bit of a big/messy diff, that basically includes a lot of cleanup and tweaks and docs to help polish the API

  • Optimize [U]Int{8, 16, 32, 64} to avoid allocating temporary arrays and byte buffers
  • Add Float32 and Float64 parsers
  • Swap the order of arguments in repeatWithLength, so it matches the order of parsing
  • Make hexBytes more forgiving to whitespace, so it works with multi-line strings
  • Remove ByteSeq alias since we already have BS
  • Re-arrange the ElemsX/ByteX/CharX parsers; give the ByteX/CharX parsers toStrings
    that match their ByteX/CharX names
  • Move prettyBytes into the main fastparse.byte API, document it
  • Add AnyElems/AnyBytes/AnyChars parser, for quickly chomping arbitrary
    fixed-length input.
  • Update existing parsers to make heavy use of AnyBytes, WordN, and IntN parsers
  • Add a ton of minimal-example docs for all the byte specific parsers
  • Add a new faux-struct parser to be the intro example for byte parsers, move UDP parser
    down into "example byte parsers" section

@lihaoyi lihaoyi force-pushed the optimize branch 9 times, most recently from c1c8972 to 01a474a Compare September 5, 2016 06:03
@lihaoyi
Copy link
Member Author

lihaoyi commented Sep 5, 2016

This is basically ready now the build is green; when #122 lands I'll squash everything and clean this up for review

@vovapolu
Copy link
Collaborator

vovapolu commented Sep 7, 2016

It's getting complicated :) There are 2 PRs and you pushed new commit here.

@lihaoyi
Copy link
Member Author

lihaoyi commented Sep 7, 2016

Oops, yeah lemme push to the other one

@lihaoyi
Copy link
Member Author

lihaoyi commented Sep 7, 2016

Ok, time to clean this up and now that #122 has landed

…leanup and

tweaks and docs to help polish the API

- Optimize `[U]Int{8, 16, 32, 64}` to avoid allocating temporary arrays and byte buffers
- Add `Float32` and `Float64` parsers
- Swap the order of arguments in `repeatWithLength`, so it matches the order of parsing
- Make `hexBytes` more forgiving to whitespace, so it works with multi-line strings
- Remove `ByteSeq` alias since we already have `BS`
- Re-arrange the `ElemsX`/`ByteX`/`CharX` parsers; give the `ByteX`/`CharX` parsers toStrings
  that match their ByteX/CharX names
- Move `prettyBytes` into the main `fastparse.byte` API, document it
- Add `AnyElems`/`AnyBytes`/`AnyChars` parser, for quickly chomping arbitrary
  fixed-length input.
- Update existing parsers to make heavy use of `AnyBytes`, `WordN`, and `IntN` parsers
- Add a ton of minimal-example docs for all the byte specific parsers
- Add a new faux-struct parser to be the intro example for byte parsers, move UDP parser
  down into "example byte parsers" section
@lihaoyi lihaoyi changed the title Optimize byte-parsers and clean up ByteApi Polish and documentation work for fastparse.byte Sep 8, 2016
s"Cannot show indices $markers outside input bounds 0 -> ${bytes.length}"
)

val maxIndexWidth = math.floor(math.max(0, math.log10(99))).toInt + 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

It equals to 2 :)

Copy link
Member Author

Choose a reason for hiding this comment

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

lol nice catch, yeah that should be bytes.length

@vovapolu
Copy link
Collaborator

vovapolu commented Sep 8, 2016

@lihaoyi New tests are wrong, and there is also some big debug output for new midi parser that distracts a little.

@lihaoyi
Copy link
Member Author

lihaoyi commented Sep 8, 2016

Oh wat, I thought I had things running green before pushing. Lemme take a look

@lihaoyi
Copy link
Member Author

lihaoyi commented Sep 8, 2016

Looks like it was a JS/JVM incompatibility; I only tested it on JVM locally. Will fix

@lihaoyi
Copy link
Member Author

lihaoyi commented Sep 8, 2016

Turns out math.log10(1000) returns 3.0 in Scala-JVM and 2.9999999999999996 in Scala.js, and this was causing the calculation of number-width to be wrong. I'll just replace it with a bytes.length.toString.length

@@ -21,7 +21,7 @@ object ByteUtils{
s"Cannot show indices $markers outside input bounds 0 -> ${bytes.length}"
)

val maxIndexWidth = math.floor(math.max(0, math.log10(bytes.length))).toInt + 1
val maxIndexWidth = bytes.length.toString.length
Copy link
Collaborator

Choose a reason for hiding this comment

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

lol, nice solution :D

@vovapolu vovapolu merged commit 6e0a7d3 into master Sep 8, 2016
@lihaoyi lihaoyi deleted the optimize branch October 27, 2018 03:57
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

2 participants