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.

Sign up for GitHub

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

Jump to bottom

Remove regexp use #40

Merged
merged 5 commits into from May 17, 2022
Merged

Remove regexp use #40

merged 5 commits into from May 17, 2022

Commits on May 16, 2022

  1. size_test: add tests for 0.3 + suffix 

    Such sizes are quite valid but were never tested.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
    @kolyshkin
    kolyshkin committed May 16, 2022
    Copy the full SHA
    af07f36 View commit details
    Browse the repository at this point in the history

  2. size_test: add parseSize benchmark 

    Shows on my laptop (6 iterations processed by benchstat):

name         time/op
ParseSize-4  10.6µs ± 3%

name         alloc/op
ParseSize-4  3.26kB ± 0%

name         allocs/op
ParseSize-4    72.0 ± 0%

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
    @kolyshkin
    kolyshkin committed May 16, 2022
    Copy the full SHA
    1d23ffa View commit details
    Browse the repository at this point in the history

  3. size_test: add t.Helper annotations 

    This way, error messages will show correct line information.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
    @kolyshkin
    kolyshkin committed May 16, 2022
    Copy the full SHA
    54c3e55 View commit details
    Browse the repository at this point in the history

  4. size_test: add more tests 

    Co-authored-by: Kir Kolyshkin <kolyshkin@gmail.com>
    @thaJeztah @kolyshkin
    thaJeztah and kolyshkin committed May 16, 2022
    Copy the full SHA
    cec4960 View commit details
    Browse the repository at this point in the history

  5. size: stop using regexp 

    Using a regular expression brings in the whole regexp and regexp/syntax
packages, which increase the resulting binary size by about 130K
(Go 1.18.1, Linux/amd64).

Besides, regular expressions are generally slow and incur some
initialization overhead (to compile all global regexp.MustComplile
variables). This, unlike the size difference, is not the main motivation
for this commit, but it feels like it should have also been mentioned.

A quick benchmark comparison shows a huge improvement (again, this is
not why this is done, nevertheless it pleases the eye):

name         old time/op    new time/op    delta
ParseSize-4    10.6µs ± 3%     2.6µs ±29%  -75.10%  (p=0.002 n=6+6)

name         old alloc/op   new alloc/op   delta
ParseSize-4    3.26kB ± 0%    0.20kB ± 0%  -93.75%  (p=0.000 n=7+6)

name         old allocs/op  new allocs/op  delta
ParseSize-4      72.0 ± 0%      26.0 ± 0%  -63.89%  (p=0.000 n=7+6)

Compatibility note: As a result, we are now a but more relaxed to the
input, allowing e.g. ".4 Gb", or "-0", or "234. B", following the rules
of strconv.ParseFloat. It seems that those were previously rejected as
a result of a regex being used, not deliberately.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
    @kolyshkin
    kolyshkin committed May 16, 2022
    Copy the full SHA
    7375726 View commit details
    Browse the repository at this point in the history