elmcraft / core-extra

Utility functions for an improved experience with elm/core
https://package.elm-lang.org/packages/elmcraft/core-extra/latest/
Other
23 stars 11 forks source link

Improve performance of String.Extra.isBlank #26

Closed Janiczek closed 1 year ago

Janiczek commented 1 year ago

This changes the implementation of String.Extra.isBlank from a regex-based one to a String.trim-using one.

This benchmark shows improvement, at least in my Chrome:

suite : Benchmark
suite =
    let
        emptyString =
            ""

        wsString =
            String.repeat 10 " "

        fullString =
            String.repeat 10 "Hello World"
    in
    Benchmark.describe "String.isBlank"
        [ Benchmark.compare "Empty string"
            "String.Extra"
            (\_ -> fn1 emptyString)
            "naive"
            (\_ -> fn2 emptyString)
        , Benchmark.compare "Whitespace string"
            "String.Extra"
            (\_ -> fn1 wsString)
            "naive"
            (\_ -> fn2 wsString)
        , Benchmark.compare "Full string"
            "String.Extra"
            (\_ -> fn1 fullString)
            "naive"
            (\_ -> fn2 fullString)
        ]

fn1 string =
    --Regex.contains (regexFromString "^\\s*$") string
    String.Extra.isBlank string

fn2 string =
    String.trim string == ""

Results

Results show between 5x and 10x speed increase.

This test passes, comparing the output of the two functions:

suite : Test
suite =
    Test.fuzz wsHeavyStringFuzzer "Oracle test for new impl of String.Extra.isBlank" <|
        \str ->
            (String.trim str == "")
                |> Expect.equal (String.Extra.isBlank str)

wsHeavyStringFuzzer : Fuzzer String
wsHeavyStringFuzzer =
    Fuzz.list wsHeavyCharFuzzer
        |> Fuzz.map String.fromList

wsHeavyCharFuzzer : Fuzzer Char
wsHeavyCharFuzzer =
    Fuzz.frequency
        [ ( 3, Fuzz.constant ' ' )
        , ( 3, Fuzz.constant '\n' )
        , ( 3, Fuzz.constant '\t' )
        , ( 3, Fuzz.constant '\u{000D}' ) -- \r
        , ( 1, Fuzz.char )
        ]
gampleman commented 1 year ago

Very nice!

Can we please add the benchmark to /benchmarks and the oracle test to the tests?

(Also this is unrelated to this PR and something I intend to do a full pass on anyway, but it would help with confidence here is to convert the docs to doctests; essentially s/==/-->).

gampleman commented 1 year ago

Thanks @Janiczek!