diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4f4ab06..e30f935 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -93,34 +93,13 @@ jobs: - uses: extractions/setup-just@53165ef7e734c5c07cb06b3c8e7b647c5aa16db3 # v4 - run: just test-32bit - # Asks the tests of both modules for their fuzz targets, so that the fuzz job cannot miss a new one. - fuzz-targets: - runs-on: ubuntu-latest - timeout-minutes: 5 - outputs: - matrix: ${{ steps.list.outputs.matrix }} - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - persist-credentials: false - - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 - with: - go-version-file: go.mod - cache: false - - uses: extractions/setup-just@53165ef7e734c5c07cb06b3c8e7b647c5aa16db3 # v4 - - id: list - run: | - matrix=$(just fuzz-matrix) - echo "matrix=$matrix" >> "$GITHUB_OUTPUT" - + # Every fuzz target of both modules, one after the other. The recipes ask the tests for their targets, so a new one + # cannot be missed. A target runs for 45 seconds on a push and for 10 minutes at night. fuzz: - needs: fuzz-targets - name: fuzz (${{ matrix.target }}, ${{ matrix.recipe }}) - strategy: - fail-fast: false - matrix: ${{ fromJSON(needs.fuzz-targets.outputs.matrix) }} runs-on: ubuntu-latest - timeout-minutes: 20 + timeout-minutes: 90 + env: + FUZZTIME: ${{ github.event_name == 'schedule' && '10m' || '45s' }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -135,23 +114,19 @@ jobs: - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: ~/.cache/go-build/fuzz - key: fuzz-${{ matrix.target }}-${{ github.run_id }} - restore-keys: fuzz-${{ matrix.target }}- - - run: just "$RECIPE" "$FUZZTIME" "$TARGET" - env: - RECIPE: ${{ matrix.recipe }} - TARGET: ${{ matrix.target }} - FUZZTIME: ${{ github.event_name == 'schedule' && '10m' || '45s' }} + key: fuzz-${{ github.run_id }} + restore-keys: fuzz- + - run: just fuzz "$FUZZTIME" + - run: just compat-fuzz "$FUZZTIME" - name: Upload the failing input if: failure() uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 with: - name: fuzz-${{ matrix.target }} + name: fuzz-failing-input path: | testdata/fuzz internal/compat/testdata/fuzz - # Too slow for every push: every change to the code runs the tests again. mutation: if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' runs-on: ubuntu-latest @@ -177,41 +152,3 @@ jobs: go-version-input: stable check-latest: true cache: false - - # Publishes the measured coverage for the README badge, also when it drops below 100%, as a shields.io endpoint file - # on the badges branch. - coverage-badge: - if: github.ref == 'refs/heads/main' && github.event_name != 'pull_request' - runs-on: ubuntu-latest - timeout-minutes: 10 - permissions: - contents: write # push the badges branch - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - persist-credentials: false - - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 - with: - go-version-file: go.mod - cache: false - - uses: extractions/setup-just@53165ef7e734c5c07cb06b3c8e7b647c5aa16db3 # v4 - - run: just coverage - - name: Publish the badge - env: - GH_TOKEN: ${{ github.token }} - run: | - total=$(go tool cover -func=coverage.out | awk '/^total:/ { print $3 }') - color=$(awk -v t="${total%\%}" 'BEGIN { - print (t >= 100 ? "brightgreen" : t >= 90 ? "green" : t >= 75 ? "yellow" : "red") - }') - mkdir "$RUNNER_TEMP/badges" - cd "$RUNNER_TEMP/badges" - printf '{"schemaVersion":1,"label":"coverage","message":"%s","color":"%s"}\n' "$total" "$color" \ - > coverage.json - # One fresh commit each time, force-pushed, so the branch never grows. - git init -q -b badges - git add coverage.json - git -c user.name="github-actions[bot]" -c user.email="41898282+github-actions[bot]@users.noreply.github.com" \ - commit -q -m "Coverage ${total} at ${GITHUB_SHA::7}" - git push -q -f "https://x-access-token:${GH_TOKEN}@github.com/${GITHUB_REPOSITORY}.git" badges - echo "### Coverage badge: ${total}" >> "$GITHUB_STEP_SUMMARY" diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml deleted file mode 100644 index c32888f..0000000 --- a/.github/workflows/scorecard.yml +++ /dev/null @@ -1,41 +0,0 @@ -# OpenSSF Scorecard rates the repository's security practices, such as pinned actions, token permissions and branch -# protection. The results go to the Security tab and, through publish_results, to the badge in the README. -name: Scorecard - -on: - push: - branches: [ main ] - schedule: - - cron: "41 5 * * 1" - branch_protection_rule: - workflow_dispatch: - -permissions: - contents: read - -jobs: - analysis: - runs-on: ubuntu-latest - timeout-minutes: 10 - permissions: - contents: read - actions: read - security-events: write # upload the results to code scanning - id-token: write # publish the results to the badge - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - persist-credentials: false - - uses: ossf/scorecard-action@fa7117db83c52d787ae5a89f350999701505d78e # v2.5.0 - with: - results_file: results.sarif - results_format: sarif - publish_results: true - - uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 - with: - name: scorecard - path: results.sarif - retention-days: 5 - - uses: github/codeql-action/upload-sarif@33d7b85da1a04171e211befe2c739303b9a4c066 # v4.39.3 - with: - sarif_file: results.sarif diff --git a/BENCHMARKS.md b/BENCHMARKS.md index bc31a23..f9e4775 100644 --- a/BENCHMARKS.md +++ b/BENCHMARKS.md @@ -1,212 +1,42 @@ # Benchmarks -What a duration costs with jsonduration, next to what the standard library takes for the same value without it. - -- **It allocates nothing of its own.** Writing and reading a duration with `encoding/json/v2` allocates what an - `int64` in the same place does: the JSON that `json.Marshal` returns, and nothing on reading. Only an error - allocates. -- **`With` is cheap to call.** `With`, `Marshalers`, and `Unmarshalers` take 5 to 13 ns and allocate nothing, so there - is no need to keep what they return. -- **A duration in a struct field takes about 50 ns more to write and 60 to 100 ns more to read than an `int64`.** - About 35 ns of that is what the standard library takes to call any function for a type. The rest is the format. -- **Wrapper types are faster than options in a struct with many fields.** With options, the standard library asks for - every field whether a function applies to it. With wrapper types it does not. - -## How they were measured - -- With Go 1.27.3 on a MacBook Pro with an Apple M2 and 16 GB of memory, macOS 27.0.1, `darwin/arm64`, in November - 2026. -- Every number is the median of ten runs of half a second each. `just bench` runs every benchmark once instead. - - ```sh - go test -run '^$' -bench . -benchmem -benchtime 500ms -count 10 - ``` - -- Times are in nanoseconds for one operation. Two runs of the same benchmark differ by one or two percent, so a few - nanoseconds between two rows mean nothing. -- The duration is 1h30m0.5s unless a section says otherwise: `"1h30m0.5s"`, `5400.5`, `5400500`, `5400500000`, - `5400500000000`, and `"PT1H30M0.5S"` in the six formats. -- The benchmarks are in [bench_test.go](bench_test.go). Each heading below names its benchmark. - -The tables compare jsonduration with four ways to do without it: - -- **`int64`.** The field is an `int64` and there are no options. This is what a number costs in the standard library. -- **Nanosecond option.** A `time.Duration` with `jsonv1.FormatDurationAsNano(true)`, the standard library's own way to - write a duration, as nanoseconds only. -- **Smallest function.** A `time.Duration` with `json.WithMarshalers` and `json.WithUnmarshalers`, and functions that - do nothing but write and read the nanoseconds as an integer. `With` passes its functions the same way, so this is - the least that it could cost. -- **Text methods.** A type of your own with `MarshalText` and `UnmarshalText`, which call `time.Duration.String` and - `time.ParseDuration`. This is what one writes by hand. - -## Building the options - -`BenchmarkOptions` - -| Call | Time | Allocations | -|---------------------------------------------------------|-----:|------------:| -| `With(ISO8601)` | 5.5 | 0 | -| `With(ISO8601, AlsoAccept(Nano, Units), AcceptDays())` | 12.8 | 0 | -| `Marshalers(ISO8601)` | 5.1 | 0 | -| `Unmarshalers(ISO8601, AlsoAccept(Nano))` | 8.7 | 0 | - -The options are built once for each set of arguments and shared afterwards. Versions up to v0.3.1 built them on -every call, which took about 260 ns and 13 allocations for `With(ISO8601)`, so keep the options in a variable there. - -## Writing one duration - -`BenchmarkMarshal`: `json.Marshal` of a pointer to a `struct{ D time.Duration }`. - -| Format | Options kept | `With` on every call | Wrapper type | -|-----------|-------------:|---------------------:|-------------:| -| `Units` | 152 | 159 | 148 | -| `Sec` | 156 | 161 | 146 | -| `Milli` | 152 | 159 | 141 | -| `Micro` | 155 | 160 | 139 | -| `Nano` | 157 | 162 | 143 | -| `ISO8601` | 157 | 161 | 141 | - -Each of these allocates once, 16 or 24 bytes: the JSON that `json.Marshal` returns. - -| Without jsonduration | Time | Allocations | -|----------------------|-----:|------------:| -| `int64` | 102 | 1 | -| Nanosecond option | 104 | 1 | -| Smallest function | 136 | 1 | -| Text methods | 151 | 2 | - -`BenchmarkMarshalWrite` writes into an `io.Writer` with `json.MarshalWrite`, which returns no JSON. Then nothing is -left to allocate: - -| Way | Time | Allocations | -|--------------------------|-----------:|------------:| -| jsonduration, any format | 145 to 162 | 0 | -| `int64` | 94 | 0 | -| Nanosecond option | 97 | 0 | -| Smallest function | 131 | 0 | -| Text methods | 154 | 1 | - -## Reading one duration - -`BenchmarkUnmarshal`: `json.Unmarshal` into the same struct. - -| Format | Options kept | `With` on every call | Wrapper type | -|-----------|-------------:|---------------------:|-------------:| -| `Units` | 207 | 208 | 182 | -| `Sec` | 180 | 187 | 153 | -| `Milli` | 174 | 174 | 145 | -| `Micro` | 177 | 178 | 149 | -| `Nano` | 180 | 181 | 153 | -| `ISO8601` | 212 | 213 | 183 | - -None of these allocates. - -| Without jsonduration | Time | Allocations | -|----------------------|-----:|------------:| -| `int64` | 115 | 0 | -| Nanosecond option | 116 | 0 | -| Smallest function | 152 | 0 | -| Text methods | 174 | 1 | - -## Reading more than one format - -`BenchmarkUnmarshalAlsoAccept` - -| Options | Input | Time | Allocations | -|-------------------------------------|-------------------|-----:|------------:| -| `With(ISO8601)` | `"PT1H30M0.5S"` | 211 | 0 | -| `With(ISO8601, AlsoAccept(Units))` | `"PT1H30M0.5S"` | 212 | 0 | -| `With(ISO8601, AlsoAccept(Units))` | `"1h30m0.5s"` | 213 | 0 | -| `With(ISO8601, AlsoAccept(Nano))` | `5400500000000` | 181 | 0 | - -A second format costs nothing that can be measured, whether the input is in the first format or in the second. - -## Many durations - -`BenchmarkSlice`: a `[]time.Duration` of a thousand durations, from 1h30m0.5s on in steps of one second. The times -are for one duration, a thousandth of the whole. - -| Way | Writing | Reading | -|-------------------|--------:|--------:| -| `Units` | 46.6 | 101.0 | -| `Sec` | 47.6 | 65.1 | -| `Milli` | 41.6 | 59.1 | -| `Micro` | 43.4 | 64.2 | -| `Nano` | 45.1 | 68.7 | -| `ISO8601` | 47.0 | 106.4 | -| `int64` | 14.8 | 33.2 | -| Nanosecond option | 16.3 | 34.4 | -| Smallest function | 27.2 | 41.9 | - -Writing the thousand allocates once, for the JSON, and reading into a slice that is long enough allocates nothing, in -every row. The number of allocations does not grow with the number of durations. - -## A struct with other fields - -`BenchmarkConfig`: nine values, five of them durations. - -```go -type config struct { - Name string - Address string - Retries int - Verbose bool - Timeout time.Duration - Interval time.Duration - Backoff []time.Duration // three of them -} -``` - -| Way | Writing, options | Writing, wrapper types | Reading, options | Reading, wrapper types | -|-------------------|-----------------:|-----------------------:|-----------------:|-----------------------:| -| `Units` | 605 | 533 | 825 | 729 | -| `Sec` | 609 | 521 | 818 | 697 | -| `Milli` | 616 | 534 | 816 | 687 | -| `Micro` | 618 | 533 | 819 | 709 | -| `Nano` | 632 | 548 | 837 | 733 | -| `ISO8601` | 581 | 495 | 908 | 800 | -| `int64` | 346 | | 535 | | -| Nanosecond option | 350 | | 536 | | -| Smallest function | 527 | | 681 | | - -Writing allocates once, for the JSON, and reading allocates nothing, in every row. - -The smallest function takes 180 ns more than `int64` to write this struct, but only 35 ns more to write the struct -with one field. That is because the standard library, once it has functions from `json.WithMarshalers` or -`json.WithUnmarshalers`, asks for every field whether one of them applies to its type, each time it writes or reads -the struct. The strings, the integer, and the boolean pay for it too. A wrapper type needs no options, so nothing is -asked, and that is why wrapper types are 70 to 130 ns faster here. - -## Invalid input - -`BenchmarkUnmarshalInvalid`: `{"D":"1h30m"}`, which is rejected. The error builds its message when `Error` is -called, so the table has the cost for a caller who only checks the error, with `errors.Is` for example, and for one -who also reads the message. - -| Way | Checked: time | Bytes | Allocations | Read: time | Bytes | Allocations | -|-----------------------------------|--------------:|------:|------------:|-----------:|------:|------------:| -| `With(ISO8601)` | 292 | 192 | 5 | 516 | 528 | 13 | -| `int64`, which rejects the string | 169 | 128 | 3 | 273 | 256 | 8 | - -An error is the one thing that allocates. Until its message is read, a syntax error like this one takes two -allocations more than the standard library's own error: the input that it quotes, and the error itself. The message -says where the duration is, quotes the input, and names the accepted formats and the cause, which the standard -library's message for an `int64` does not. - -## Without JSON - -`BenchmarkFormat` and `BenchmarkParse`: `Format.Format` and `Format.Parse`, next to the two functions of package -`time`. - -| Format | `Format` | Bytes | Allocations | `Parse` | Allocations | -|----------------------------------------------|---------:|------:|------------:|--------:|------------:| -| `Units` | 29.9 | 16 | 1 | 59.9 | 0 | -| `Sec` | 19.6 | 8 | 1 | 20.5 | 0 | -| `Milli` | 19.4 | 8 | 1 | 20.8 | 0 | -| `Micro` | 21.6 | 16 | 1 | 22.1 | 0 | -| `Nano` | 22.8 | 16 | 1 | 24.1 | 0 | -| `ISO8601` | 31.5 | 16 | 1 | 58.7 | 0 | -| `time.Duration.String`, `time.ParseDuration` | 22.7 | 16 | 1 | 45.0 | 0 | - -`Format` allocates the string that it returns. `Parse` allocates nothing for input of up to 32 bytes and once for -longer input. Nothing that `Format` writes is longer than that. +## What the tests enforce + +These hold on every platform and Go version that CI runs, because a test fails otherwise: + +- **Writing and reading a duration allocates nothing of its own.** What is allocated is what the standard library + allocates for an `int64` in the same place: the JSON that `json.Marshal` returns, and nothing on reading. +- **`With`, `Marshalers`, and `Unmarshalers` allocate nothing without decoding options.** The options of each format + are built once, so there is no need to keep what these return. With `AlsoAccept` or `AcceptDays` they build new + options on every call, so keep those in a variable. Versions up to v0.3.1 built every set of options anew. +- **`Format.Format` allocates the string it returns, and `Format.Parse` nothing** for what `Format.Format` writes. + +Only an error allocates. It builds its message when `Error` is called, so a caller who only checks it with +`errors.Is` does not pay for the message. + +## Roughly how long it takes + +One duration of 1h30m0.5s in a `struct{ D time.Duration }`, in nanoseconds, with Go 1.27.3 on an Apple M2 in November +2026. This is one measurement on one machine and will drift, so take the proportions from it and not the numbers. + +| Way | Writing | Reading | +|----------------------------------------------------------------|-----------:|-----------:| +| An `int64` field, without options | 102 | 115 | +| A function that does nothing but write or read the nanoseconds | 136 | 152 | +| A type of your own with `MarshalText` and `UnmarshalText` | 151 | 174 | +| `With`, in the six formats | 152 to 157 | 174 to 212 | +| A wrapper type such as `ISO8601Duration`, in the six formats | 139 to 148 | 145 to 183 | + +- **A large part of the difference to an `int64` is the standard library's.** The second row passes its function the + way `With` does, with `json.WithMarshalers` and `json.WithUnmarshalers`, so it is the least that `With` could cost. +- **A type of your own is no faster.** It also allocates once more than the other rows, each way. +- **Wrapper types are faster than options in a struct with many fields.** Once it has functions, the standard library + asks for every field whether one of them applies to its type, so the fields that are not durations pay too. A + wrapper type needs no options. +- **A second format costs nothing that can be measured.** Reading with `AlsoAccept` takes as long as without, whether + the input is in the first format or in the second. + +## Measuring it yourself + +`just bench` runs the benchmarks in [bench_test.go](bench_test.go): writing and reading one duration, `AlsoAccept`, +a struct with other fields, and `Format` and `Parse`, each next to the ways above to do without the package. diff --git a/README.md b/README.md index 169d972..049c9fd 100644 --- a/README.md +++ b/README.md @@ -2,8 +2,6 @@ [![CI](https://github.com/pscheid92/jsonduration/actions/workflows/ci.yml/badge.svg)](https://github.com/pscheid92/jsonduration/actions/workflows/ci.yml) [![Go Reference](https://pkg.go.dev/badge/github.com/pscheid92/jsonduration.svg)](https://pkg.go.dev/github.com/pscheid92/jsonduration) -[![Coverage](https://img.shields.io/endpoint?url=https%3A%2F%2Fraw.githubusercontent.com%2Fpscheid92%2Fjsonduration%2Fbadges%2Fcoverage.json)](https://github.com/pscheid92/jsonduration/actions/workflows/ci.yml) -[![OpenSSF Scorecard](https://api.scorecard.dev/projects/github.com/pscheid92/jsonduration/badge)](https://scorecard.dev/viewer/?uri=github.com/pscheid92/jsonduration) JSON formats for `time.Duration` in Go's `encoding/json/v2`. @@ -55,8 +53,9 @@ func main() { } ``` -`encoding/json/v2` is part of the standard library since Go 1.27, with no `GOEXPERIMENT` setting. `With` builds the -options on the first call and returns the same ones afterwards, so a later call costs a lookup and no allocation. +`encoding/json/v2` is part of the standard library since Go 1.27, with no `GOEXPERIMENT` setting. Without decoding +options, `With` returns the same options on every call and allocates nothing. With `AlsoAccept` or `AcceptDays` it +builds new ones, so keep those in a variable. If you already pass `json.WithMarshalers` or `json.WithUnmarshalers`, don't add `With` next to them: a second `WithMarshalers` option replaces the first, without an error. Join yours with the package's instead: @@ -208,7 +207,9 @@ flag.Parse() opts := jsonduration.With(format) ``` -`With` panics on an invalid `Format`, which includes the empty one. Check a `Format` from a plain string first: +A `Format` is its name, which is what lets a flag or a configuration hold one. So a `Format` that names nothing can +exist, like `time.Month(13)`, and using it is a programming error: `With` panics on an invalid `Format`, which +includes the empty one. Check a `Format` from a plain string first: ```go format := jsonduration.Format(os.Getenv("DURATION_FORMAT")) @@ -233,12 +234,13 @@ d, err := jsonduration.Sec.Parse("5400.5") // 1h30m0.5s - To leave out zero durations, use `omitzero`. In v2, `omitempty` keeps them. - Decoding checks the format, not the value: `"-1s"` and `"0s"` are read. Check ranges after decoding. - Numeric formats reject exponents, as in `1e-05`, which some encoders such as Swift's write for small numbers. + That is on purpose: decoding is strict, and exponents would add a second spelling of every number for a rare case. - Digits below a nanosecond are truncated, not rounded. No format rounds to whole units: `Milli` writes `1.234567`. - The largest duration is about 292 years, `PT2562047H47M16.854775807S`; anything larger is `strconv.ErrRange`. - `ISO8601` takes one sign, `-` or `+`, only in front, as in `-PT30S`. It rejects Java's negative durations, such as - `PT-30S`. It reads designators in either case and a decimal comma, so `pt1,5s` is 1.5s. A count of zero must have - no fraction: `"P0D"` is read, `"P0.0D"` is not. Output is always the shortest form, so `"P0DT2H"` comes back as - `"PT2H"`. + `PT-30S`, as the `format` tag of `encoding/json/v2` did. It reads designators in either case and a decimal comma, + so `pt1,5s` is 1.5s. A count of zero must have no fraction: `"P0D"` is read, `"P0.0D"` is not. Output is always the + shortest form, so `"P0DT2H"` comes back as `"PT2H"`. - `Units` reads what `time.ParseDuration` reads. So it reads protobuf's `"3600s"` but writes `"1h0m0s"`, and it does not read Rust humantime's `"1h 30m"` or PostgreSQL's `"01:30:00"`. PostgreSQL writes ISO 8601 with `intervalstyle = iso_8601`. @@ -263,9 +265,8 @@ them. [TESTING.md](TESTING.md) has the details. ## How fast it is jsonduration allocates nothing of its own: writing and reading a duration allocates what an `int64` in the same place -does. A duration in a struct field takes about 50 ns more to write and 60 to 100 ns more to read than an `int64`, on -an Apple M2. `With` takes 5 ns, so there is no need to keep what it returns. In a struct with many fields, wrapper -types are faster than options. [BENCHMARKS.md](BENCHMARKS.md) has the numbers and what they are compared with. +does, and `With` without decoding options allocates nothing. Tests enforce both. In a struct with many fields, +wrapper types are faster than options. [BENCHMARKS.md](BENCHMARKS.md) has rough timings and the reasons. ## Development diff --git a/bench_test.go b/bench_test.go index 44f1674..d7668b6 100644 --- a/bench_test.go +++ b/bench_test.go @@ -1,10 +1,8 @@ package jsonduration import ( - jsonv1 "encoding/json" "encoding/json/jsontext" "encoding/json/v2" - "errors" "io" "testing" "time" @@ -53,43 +51,6 @@ func TestAllocations(t *testing.T) { } } -// An error allocates the input it quotes and itself. Its message is built when asked for, which this does not do. -func TestRejectedDurationAllocations(t *testing.T) { - var err error - - parse := func(f Format, in string) func() { - return func() { _, err = f.Parse(in) } - } - - // An unknown unit is an error of its own inside the other, with the name of the unit. - tests := []struct { - name string - want float64 - fn func() - }{ - {"ISO8601.Parse", 2, parse(ISO8601, "1h30m")}, - {"Units.Parse with an unknown unit", 4, parse(Units, "5fortnights")}, - } - - for _, tt := range tests { - if allocs := testing.AllocsPerRun(100, tt.fn); allocs != tt.want || err == nil { - t.Errorf("%s: %v allocs with error %v, want %v allocs and an error", tt.name, allocs, err, tt.want) - } - } - - // What encoding/json/v2 allocates around the error is its own business, and differs between platforms. - // So this measures the decoder alone, which builds the names of its formats once and not for every error. - for _, opts := range [][]DecodeOption{nil, {AlsoAccept(Nano, Units), AcceptDays()}} { - dc := newDecoder(ISO8601, configFrom(opts)) - in := jsontext.Value(`"PT1X"`) - - allocs := testing.AllocsPerRun(100, func() { _, err = dc.parse(in, false) }) - if allocs != 2 || !errors.Is(err, ErrInvalidDuration) { - t.Errorf("decoder in format %s: %v allocs with error %v, want 2 allocs and an error", dc.names, allocs, err) - } - } -} - // The benchmarks below are the ones BENCHMARKS.md reports. Their names are //, // with "baseline" in place of the format for what to compare with. @@ -113,11 +74,8 @@ func (d *textDuration) UnmarshalText(b []byte) error { return err } -// nanoOption is the option of the standard library that writes a time.Duration as nanoseconds. -var nanoOption = jsonv1.FormatDurationAsNano(true) - -// nanoFuncs does the same with functions, as With does, and does as little in them as a function can. What it costs -// more than nanoOption is what the standard library takes to call a function for a type. +// nanoFuncs passes functions as With does, and does as little in them as a function can: it writes and reads +// nanoseconds. What it costs more than an int64 is what the standard library takes to call a function for a type. var nanoFuncs = json.JoinOptions( json.WithMarshalers(json.MarshalToFunc(func(enc *jsontext.Encoder, d time.Duration) error { return enc.WriteToken(jsontext.Int(int64(d))) @@ -159,48 +117,6 @@ func configOf[T any](wrap func(time.Duration) T) config[T] { } } -// benchWrapperConfig measures a config whose durations have the wrapper type of format f: decoding in, or encoding -// such a config if in is nil. -func benchWrapperConfig(b *testing.B, f Format, in []byte) { - b.Helper() - - switch f { - case Units: - benchConfigOf(b, func(d time.Duration) UnitsDuration { return UnitsDuration{d} }, in) - case Sec: - benchConfigOf(b, func(d time.Duration) SecDuration { return SecDuration{d} }, in) - case Milli: - benchConfigOf(b, func(d time.Duration) MilliDuration { return MilliDuration{d} }, in) - case Micro: - benchConfigOf(b, func(d time.Duration) MicroDuration { return MicroDuration{d} }, in) - case Nano: - benchConfigOf(b, func(d time.Duration) NanoDuration { return NanoDuration{d} }, in) - case ISO8601: - benchConfigOf(b, func(d time.Duration) ISO8601Duration { return ISO8601Duration{d} }, in) - } -} - -func benchConfigOf[T any](b *testing.B, wrap func(time.Duration) T, in []byte) { - b.Helper() - - if in == nil { - benchMarshal(b, configOf(wrap)) - return - } - - benchUnmarshal[config[T]](b, in) -} - -// manyDurations returns n durations of different lengths, about as long as d90. -func manyDurations(n int) []time.Duration { - durations := make([]time.Duration, n) - for i := range durations { - durations[i] = d90 + time.Duration(i)*time.Second - } - - return durations -} - // benchMarshal measures json.Marshal of a pointer to v, so that handing v over does not allocate. func benchMarshal[T any](b *testing.B, v T, opts ...json.Options) { b.Helper() @@ -213,18 +129,6 @@ func benchMarshal[T any](b *testing.B, v T, opts ...json.Options) { } } -// benchMarshalWrite measures json.MarshalWrite, which returns no JSON and so does not allocate for it. -func benchMarshalWrite[T any](b *testing.B, v T, opts ...json.Options) { - b.Helper() - b.ReportAllocs() - - for b.Loop() { - if err := json.MarshalWrite(io.Discard, &v, opts...); err != nil { - b.Fatal(err) - } - } -} - // benchUnmarshal measures json.Unmarshal of in into a T, which is reused between calls. func benchUnmarshal[T any](b *testing.B, in []byte, opts ...json.Options) { b.Helper() @@ -250,80 +154,54 @@ func mustMarshal(b *testing.B, v any, opts ...json.Options) []byte { return out } -// benchWrapper measures a field of the wrapper type of format f: decoding in, or encoding such a field if in is nil. -func benchWrapper(b *testing.B, f Format, in []byte) { +// benchValue measures decoding in into a T, or encoding v if in is nil. +func benchValue[T any](b *testing.B, v T, in []byte, opts ...json.Options) { + b.Helper() + + if in == nil { + benchMarshal(b, v, opts...) + return + } + + benchUnmarshal[T](b, in, opts...) +} + +// benchWrapper measures a value whose durations have the wrapper type of format f: a field, or a whole config. +// It decodes in, or encodes the value if in is nil. +func benchWrapper(b *testing.B, f Format, whole bool, in []byte) { b.Helper() switch f { case Units: - benchField(b, UnitsDuration{d90}, in) + benchWrapped(b, func(d time.Duration) UnitsDuration { return UnitsDuration{d} }, whole, in) case Sec: - benchField(b, SecDuration{d90}, in) + benchWrapped(b, func(d time.Duration) SecDuration { return SecDuration{d} }, whole, in) case Milli: - benchField(b, MilliDuration{d90}, in) + benchWrapped(b, func(d time.Duration) MilliDuration { return MilliDuration{d} }, whole, in) case Micro: - benchField(b, MicroDuration{d90}, in) + benchWrapped(b, func(d time.Duration) MicroDuration { return MicroDuration{d} }, whole, in) case Nano: - benchField(b, NanoDuration{d90}, in) + benchWrapped(b, func(d time.Duration) NanoDuration { return NanoDuration{d} }, whole, in) case ISO8601: - benchField(b, ISO8601Duration{d90}, in) + benchWrapped(b, func(d time.Duration) ISO8601Duration { return ISO8601Duration{d} }, whole, in) } } -// benchField measures decoding in into a field of the type of d, or encoding such a field if in is nil. -func benchField[T any](b *testing.B, d T, in []byte) { +func benchWrapped[T any](b *testing.B, wrap func(time.Duration) T, whole bool, in []byte) { b.Helper() - if in == nil { - benchMarshal(b, field[T]{d}) + if whole { + benchValue(b, configOf(wrap), in) return } - benchUnmarshal[field[T]](b, in) -} - -// options keeps results of With alive, so that the calls are not optimized away. -var ( - options json.Options - marshalers *json.Marshalers - unmarshalers *json.Unmarshalers -) - -func BenchmarkOptions(b *testing.B) { - b.Run("With", func(b *testing.B) { - b.ReportAllocs() - for b.Loop() { - options = With(ISO8601) - } - }) - - b.Run("With_AlsoAccept_AcceptDays", func(b *testing.B) { - b.ReportAllocs() - for b.Loop() { - options = With(ISO8601, AlsoAccept(Nano, Units), AcceptDays()) - } - }) - - b.Run("Marshalers", func(b *testing.B) { - b.ReportAllocs() - for b.Loop() { - marshalers = Marshalers(ISO8601) - } - }) - - b.Run("Unmarshalers", func(b *testing.B) { - b.ReportAllocs() - for b.Loop() { - unmarshalers = Unmarshalers(ISO8601, AlsoAccept(Nano)) - } - }) + benchValue(b, field[T]{wrap(d90)}, in) } func BenchmarkMarshal(b *testing.B) { v := field[time.Duration]{d90} b.Run("baseline/int64", func(b *testing.B) { benchMarshal(b, field[int64]{int64(d90)}) }) - b.Run("baseline/nano_option", func(b *testing.B) { benchMarshal(b, v, nanoOption) }) b.Run("baseline/nano_funcs", func(b *testing.B) { benchMarshal(b, v, nanoFuncs) }) b.Run("baseline/text_methods", func(b *testing.B) { benchMarshal(b, field[textDuration]{textDuration(d90)}) }) @@ -331,39 +209,15 @@ func BenchmarkMarshal(b *testing.B) { opts := With(f) b.Run(string(f)+"/options", func(b *testing.B) { benchMarshal(b, v, opts) }) - b.Run(string(f)+"/wrapper", func(b *testing.B) { benchWrapper(b, f, nil) }) - - b.Run(string(f)+"/With_per_call", func(b *testing.B) { - b.ReportAllocs() - for b.Loop() { - if _, err := json.Marshal(&v, With(f)); err != nil { - b.Fatal(err) - } - } - }) - } -} - -func BenchmarkMarshalWrite(b *testing.B) { - v := field[time.Duration]{d90} - - b.Run("baseline/int64", func(b *testing.B) { benchMarshalWrite(b, field[int64]{int64(d90)}) }) - b.Run("baseline/nano_option", func(b *testing.B) { benchMarshalWrite(b, v, nanoOption) }) - b.Run("baseline/nano_funcs", func(b *testing.B) { benchMarshalWrite(b, v, nanoFuncs) }) - b.Run("baseline/text_methods", func(b *testing.B) { benchMarshalWrite(b, field[textDuration]{textDuration(d90)}) }) - - for _, f := range Formats() { - opts := With(f) - b.Run(string(f)+"/options", func(b *testing.B) { benchMarshalWrite(b, v, opts) }) + b.Run(string(f)+"/wrapper", func(b *testing.B) { benchWrapper(b, f, false, nil) }) } } func BenchmarkUnmarshal(b *testing.B) { v := field[time.Duration]{d90} - nanos := mustMarshal(b, v, nanoOption) + nanos := mustMarshal(b, field[int64]{int64(d90)}) b.Run("baseline/int64", func(b *testing.B) { benchUnmarshal[field[int64]](b, nanos) }) - b.Run("baseline/nano_option", func(b *testing.B) { benchUnmarshal[field[time.Duration]](b, nanos, nanoOption) }) b.Run("baseline/nano_funcs", func(b *testing.B) { benchUnmarshal[field[time.Duration]](b, nanos, nanoFuncs) }) b.Run("baseline/text_methods", func(b *testing.B) { benchUnmarshal[field[textDuration]](b, mustMarshal(b, v, With(Units))) @@ -374,25 +228,15 @@ func BenchmarkUnmarshal(b *testing.B) { in := mustMarshal(b, v, opts) b.Run(string(f)+"/options", func(b *testing.B) { benchUnmarshal[field[time.Duration]](b, in, opts) }) - b.Run(string(f)+"/wrapper", func(b *testing.B) { benchWrapper(b, f, in) }) - - b.Run(string(f)+"/With_per_call", func(b *testing.B) { - b.ReportAllocs() - - var got field[time.Duration] - for b.Loop() { - if err := json.Unmarshal(in, &got, With(f)); err != nil { - b.Fatal(err) - } - } - }) + b.Run(string(f)+"/wrapper", func(b *testing.B) { benchWrapper(b, f, false, in) }) } } // BenchmarkUnmarshalAlsoAccept measures what a second accepted format costs. func BenchmarkUnmarshalAlsoAccept(b *testing.B) { v := field[time.Duration]{d90} - iso, units, nanos := mustMarshal(b, v, With(ISO8601)), mustMarshal(b, v, With(Units)), mustMarshal(b, v, nanoOption) + iso, units := mustMarshal(b, v, With(ISO8601)), mustMarshal(b, v, With(Units)) + nanos := mustMarshal(b, field[int64]{int64(d90)}) isoOnly := With(ISO8601) isoOrUnits := With(ISO8601, AlsoAccept(Units)) @@ -409,82 +253,16 @@ func BenchmarkUnmarshalAlsoAccept(b *testing.B) { b.Run("second_is_a_number", func(b *testing.B) { benchUnmarshal[field[time.Duration]](b, nanos, isoOrNano) }) } -// benchRejected measures json.Unmarshal of in into a T, which must fail, and with message also reading the error. -func benchRejected[T any](b *testing.B, in []byte, message bool, opts ...json.Options) { - b.Helper() - b.ReportAllocs() - - var got T - for b.Loop() { - err := json.Unmarshal(in, &got, opts...) - if err == nil { - b.Fatal("no error") - } - - if message { - formatted = err.Error() - } - } -} - -// BenchmarkUnmarshalInvalid measures a duration that is rejected. The error builds its message when asked, -// so there is one benchmark for a caller who only checks the error and one for a caller who reads it. -func BenchmarkUnmarshalInvalid(b *testing.B) { - in := []byte(`{"D":"1h30m"}`) - opts := With(ISO8601) - - b.Run("options/checked", func(b *testing.B) { benchRejected[field[time.Duration]](b, in, false, opts) }) - b.Run("options/read", func(b *testing.B) { benchRejected[field[time.Duration]](b, in, true, opts) }) - b.Run("baseline/int64/checked", func(b *testing.B) { benchRejected[field[int64]](b, in, false) }) - b.Run("baseline/int64/read", func(b *testing.B) { benchRejected[field[int64]](b, in, true) }) -} - -// BenchmarkSlice measures a thousand durations in one value, to show the cost of each and that allocations do not -// grow with their number. -func BenchmarkSlice(b *testing.B) { - const n = 1000 - - durations := manyDurations(n) - ints := make([]int64, n) - for i, d := range durations { - ints[i] = int64(d) - } - - nanos := mustMarshal(b, ints) - - b.Run("Marshal/baseline/int64", func(b *testing.B) { benchMarshal(b, ints) }) - b.Run("Marshal/baseline/nano_option", func(b *testing.B) { benchMarshal(b, durations, nanoOption) }) - b.Run("Marshal/baseline/nano_funcs", func(b *testing.B) { benchMarshal(b, durations, nanoFuncs) }) - b.Run("Unmarshal/baseline/int64", func(b *testing.B) { benchUnmarshal[[]int64](b, nanos) }) - b.Run("Unmarshal/baseline/nano_option", func(b *testing.B) { - benchUnmarshal[[]time.Duration](b, nanos, nanoOption) - }) - b.Run("Unmarshal/baseline/nano_funcs", func(b *testing.B) { - benchUnmarshal[[]time.Duration](b, nanos, nanoFuncs) - }) - - for _, f := range Formats() { - opts := With(f) - in := mustMarshal(b, durations, opts) - - b.Run("Marshal/"+string(f)+"/options", func(b *testing.B) { benchMarshal(b, durations, opts) }) - b.Run("Unmarshal/"+string(f)+"/options", func(b *testing.B) { benchUnmarshal[[]time.Duration](b, in, opts) }) - } -} - -// BenchmarkConfig measures a value with other fields than durations, to show what share the durations have. +// BenchmarkConfig measures a value with other fields than durations. With options, the standard library asks for +// every field whether a function applies to it, which a wrapper type avoids. func BenchmarkConfig(b *testing.B) { durations := configOf(func(d time.Duration) time.Duration { return d }) ints := configOf(func(d time.Duration) int64 { return int64(d) }) nanos := mustMarshal(b, ints) b.Run("Marshal/baseline/int64", func(b *testing.B) { benchMarshal(b, ints) }) - b.Run("Marshal/baseline/nano_option", func(b *testing.B) { benchMarshal(b, durations, nanoOption) }) b.Run("Marshal/baseline/nano_funcs", func(b *testing.B) { benchMarshal(b, durations, nanoFuncs) }) b.Run("Unmarshal/baseline/int64", func(b *testing.B) { benchUnmarshal[config[int64]](b, nanos) }) - b.Run("Unmarshal/baseline/nano_option", func(b *testing.B) { - benchUnmarshal[config[time.Duration]](b, nanos, nanoOption) - }) b.Run("Unmarshal/baseline/nano_funcs", func(b *testing.B) { benchUnmarshal[config[time.Duration]](b, nanos, nanoFuncs) }) @@ -494,22 +272,15 @@ func BenchmarkConfig(b *testing.B) { in := mustMarshal(b, durations, opts) b.Run("Marshal/"+string(f)+"/options", func(b *testing.B) { benchMarshal(b, durations, opts) }) - b.Run("Marshal/"+string(f)+"/wrapper", func(b *testing.B) { benchWrapperConfig(b, f, nil) }) + b.Run("Marshal/"+string(f)+"/wrapper", func(b *testing.B) { benchWrapper(b, f, true, nil) }) b.Run("Unmarshal/"+string(f)+"/options", func(b *testing.B) { benchUnmarshal[config[time.Duration]](b, in, opts) }) - b.Run("Unmarshal/"+string(f)+"/wrapper", func(b *testing.B) { benchWrapperConfig(b, f, in) }) + b.Run("Unmarshal/"+string(f)+"/wrapper", func(b *testing.B) { benchWrapper(b, f, true, in) }) } } func BenchmarkFormat(b *testing.B) { - b.Run("baseline/Duration.String", func(b *testing.B) { - b.ReportAllocs() - for b.Loop() { - formatted = d90.String() - } - }) - for _, f := range Formats() { b.Run(string(f), func(b *testing.B) { b.ReportAllocs() @@ -521,17 +292,6 @@ func BenchmarkFormat(b *testing.B) { } func BenchmarkParse(b *testing.B) { - b.Run("baseline/time.ParseDuration", func(b *testing.B) { - s := d90.String() - - b.ReportAllocs() - for b.Loop() { - if _, err := time.ParseDuration(s); err != nil { - b.Fatal(err) - } - } - }) - for _, f := range Formats() { s := f.Format(d90) diff --git a/decoder.go b/decoder.go index a28db9e..c238679 100644 --- a/decoder.go +++ b/decoder.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "reflect" + "slices" "strconv" "time" ) @@ -31,7 +32,7 @@ type decoder struct { func newDecoder(f Format, config decodeConfig) *decoder { formats := []Format{f} for _, g := range Formats() { - if g != f && config.accepted.has(g) { + if g != f && slices.Contains(config.accepted, g) { formats = append(formats, g) } } diff --git a/errors_test.go b/errors_test.go index 5d12bd0..7e71a9a 100644 --- a/errors_test.go +++ b/errors_test.go @@ -33,6 +33,8 @@ func TestInvalidFormatPanics(t *testing.T) { want string }{ {"With", func() { With(bogus) }, "invalid format"}, + {"With the empty format", func() { With("") }, `invalid format ""`}, + {"With an invalid format after a valid option", func() { With(bogus, AlsoAccept(Nano)) }, "invalid format"}, {"Marshalers", func() { Marshalers(bogus) }, "invalid format"}, {"Unmarshalers", func() { Unmarshalers(bogus) }, "invalid format"}, {"Format", func() { bogus.Format(time.Second) }, "invalid format"}, diff --git a/format.go b/format.go index da9c564..6a0f509 100644 --- a/format.go +++ b/format.go @@ -5,7 +5,8 @@ import "time" // Format is a way to write a [time.Duration] in JSON, such as [ISO8601] for "PT1H30M". // Use it with [With], or convert single durations with [Format.Format] and [Format.Parse]. // -// A flag ([flag.TextVar]) or a JSON configuration can name a format, and both reject unknown names. +// A Format is its name, so that a flag ([flag.TextVar]) or a JSON configuration can hold one, and both reject unknown +// names. As with time.Month(13), a value that names no format can therefore exist, and using it is a programming error: // [With] panics on an invalid Format, such as the empty one that decoding leaves for a missing name or null, // so give it a default, as in cmp.Or(cfg.DurationFormat, jsonduration.ISO8601). // A Format converted from a string, as in Format(os.Getenv("DURATION_FORMAT")), is not checked, @@ -40,9 +41,6 @@ const ( ISO8601 Format = "iso8601" ) -// numFormats is the number of formats. TestFormatNumbers checks it. -const numFormats = 6 - // Formats returns all formats, for example to list them in a flag's usage. func Formats() []Format { return []Format{Units, Sec, Milli, Micro, Nano, ISO8601} @@ -180,44 +178,6 @@ func (f Format) codec() (codec, bool) { return codec{}, false } -// number returns the place of f in [Formats], counted from one, and zero if f is not valid. -func (f Format) number() uint8 { - switch f { - case Units: - return 1 - case Sec: - return 2 - case Milli: - return 3 - case Micro: - return 4 - case Nano: - return 5 - case ISO8601: - return 6 - } - - return 0 -} - -// A formatSet holds formats as bits, so that a set fits into one byte and has one value whatever order it was built in. -type formatSet uint8 - -// bit returns the set of f alone, and the empty set if f is not valid. -func (f Format) bit() formatSet { - number := f.number() - if number == 0 { - return 0 - } - - return 1 << (number - 1) -} - -// has reports whether f is in the set. -func (s formatSet) has(f Format) bool { - return s&f.bit() != 0 -} - // mustCodec returns the codec of format f, and panics if f is not valid. func (f Format) mustCodec() codec { c, ok := f.codec() diff --git a/format_test.go b/format_test.go index cbaab77..5b9ecc2 100644 --- a/format_test.go +++ b/format_test.go @@ -263,23 +263,3 @@ func FuzzParse(f *testing.F) { } }) } - -// TestFormatNumbers checks that number counts the formats in the order of Formats, and that numFormats is their count. -func TestFormatNumbers(t *testing.T) { - formats := Formats() - if len(formats) != numFormats { - t.Errorf("numFormats = %d, want %d as Formats returns", numFormats, len(formats)) - } - - for i, f := range formats { - if got := int(f.number()); got != i+1 { - t.Errorf("%s.number() = %d, want %d", f, got, i+1) - } - } - - for _, f := range []Format{"", "bogus", "ISO8601"} { - if got := f.number(); got != 0 { - t.Errorf("Format(%q).number() = %d, want 0", string(f), got) - } - } -} diff --git a/json.go b/json.go index 89eafe2..0777d60 100644 --- a/json.go +++ b/json.go @@ -3,6 +3,7 @@ package jsonduration import ( "encoding/json/jsontext" "encoding/json/v2" + "slices" "time" ) @@ -16,10 +17,11 @@ import ( // in object names and with json.StringifyNumbers. JSON null sets the duration to zero. // // The options are safe for concurrent use. -// With builds them on the first call and returns the same ones for the same arguments afterwards, -// so a call costs a lookup and no allocation, and it makes no difference whether you keep the options or ask again: +// Without decoding options, With returns the same ones on every call and allocates nothing, +// so it makes no difference whether you keep them or ask again. With decoding options it builds new ones, +// so keep those: // -// var jsonOpts = jsonduration.With(jsonduration.ISO8601) +// var jsonOpts = jsonduration.With(jsonduration.ISO8601, jsonduration.AlsoAccept(jsonduration.Nano)) // // b, err := json.Marshal(v, jsonOpts) // err = json.Unmarshal(b, &v, jsonOpts) @@ -45,7 +47,6 @@ func Unmarshalers(f Format, opts ...DecodeOption) *json.Unmarshalers { // [AlsoAccept] adds formats, and [AcceptDays] reads ISO 8601 days. Writing always uses the one format given. type DecodeOption interface { // apply returns the configuration with the option applied. - // It takes a value, not a pointer, so that the configuration of a call can stay on the stack. apply(decodeConfig) decodeConfig } @@ -62,17 +63,14 @@ type DecodeOption interface { // // The order of the formats makes no difference. AlsoAccept panics if a format is not valid. func AlsoAccept(formats ...Format) DecodeOption { - var accepted formatSet for _, f := range formats { - bit := f.bit() - if bit == 0 { + if !f.Valid() { panic(errInvalidFormat(f)) } - - accepted |= bit } - return alsoAccept(accepted) + // A copy, so that the option does not change if the caller changes the slice afterwards. + return alsoAccept(slices.Clone(formats)) } // AcceptDays makes [ISO8601] decoding read a day as exactly 24 hours and a week as 7 days: "P1DT12H" is 36h. @@ -86,9 +84,9 @@ func AcceptDays() DecodeOption { return acceptDays{} } -// decodeConfig is what the options of a call add up to. +// decodeConfig is what the options of a call add up to. accepted may name a format more than once, in any order. type decodeConfig struct { - accepted formatSet + accepted []Format acceptDays bool } @@ -104,11 +102,11 @@ func configFrom(opts []DecodeOption) decodeConfig { return config } -// alsoAccept is the option of [AlsoAccept]. It is one byte, which Go puts into an interface without allocating. -type alsoAccept formatSet +// alsoAccept is the option of [AlsoAccept]. +type alsoAccept []Format func (a alsoAccept) apply(c decodeConfig) decodeConfig { - c.accepted |= formatSet(a) + c.accepted = append(c.accepted, a...) return c } diff --git a/justfile b/justfile index edbf17b..1469e90 100644 --- a/justfile +++ b/justfile @@ -77,14 +77,6 @@ compat: compat-fuzz time="30s" *targets=all_fuzz_targets: for t in {{ targets }}; do go test -run='^$' -fuzz="^$t\$" -fuzztime={{ time }} . || exit 1; done -# The fuzz targets of both modules as a matrix for GitHub Actions: CI's fuzz job runs whatever this prints. -fuzz-matrix: - #!/usr/bin/env bash - set -euo pipefail - list() { go test -list '^Fuzz' . | grep '^Fuzz' | sed "s/$/ $1/"; } - { list fuzz; cd internal/compat; list compat-fuzz; } | - jq -R -c -s 'split("\n") | map(select(. != "") | split(" ") | {target: .[0], recipe: .[1]}) | {include: .}' - bench: go test -run='^$' -bench=. -benchmem ./... diff --git a/shared.go b/shared.go index b6f650e..6c7961f 100644 --- a/shared.go +++ b/shared.go @@ -4,66 +4,64 @@ import ( "encoding/json/jsontext" "encoding/json/v2" "reflect" - "sync/atomic" "time" ) // shared holds what [With], [Marshalers] and [Unmarshalers] return for one set of arguments. -// Its values never change and are safe for concurrent use, so every call with these arguments gets the same ones. -// That saves more than building them: encoding/json/v2 remembers per value which types it has seen. +// Its values never change and are safe for concurrent use. type shared struct { marshalers *json.Marshalers unmarshalers *json.Unmarshalers options json.Options } -// sharedByArgs holds the shared values of every set of arguments: by the number of the written format less one, -// by the set of the other accepted formats, and by whether days are accepted. -// A set of arguments that is not valid, such as one with two numeric formats, keeps its nil pointer. -// An array needs no hashing and no lock, and at 768 pointers it is small. -var sharedByArgs [numFormats][1 << numFormats][2]atomic.Pointer[shared] +// sharedByFormat holds the values of each format without decoding options, which is how most calls ask for them. +// They are built once, so that such a call allocates nothing and every caller gets the same ones: +// encoding/json/v2 remembers per value which types it has seen. +var sharedByFormat = func() map[Format]*shared { + byFormat := make(map[Format]*shared, len(Formats())) + for _, f := range Formats() { + byFormat[f] = newShared(f, decodeConfig{}, nil) + } -// sharedFor returns the shared values for the arguments of [With], and builds them on the first call. -// It panics as [With] does. -func sharedFor(f Format, opts []DecodeOption) *shared { - config := configFrom(opts) + return byFormat +}() - number := f.number() - if number == 0 { +// sharedFor returns the values for the arguments of [With]: the ones of the format if no option changes anything, +// and new ones otherwise. It panics as [With] does. +func sharedFor(f Format, opts []DecodeOption) *shared { + plain, ok := sharedByFormat[f] + if !ok { panic(errInvalidFormat(f)) } - // Accepting the written format again changes nothing, so it gets no entry of its own. - config.accepted &^= f.bit() - - days := 0 - if config.acceptDays { - days = 1 + config := configFrom(opts) + if len(config.accepted) == 0 && !config.acceptDays { + return plain } - slot := &sharedByArgs[number-1][config.accepted][days] - if s := slot.Load(); s != nil { - return s - } + // Options change only how durations are read, so the marshalers are those of the format alone. + return newShared(f, config, plain.marshalers) +} - // This panics for arguments that are not valid, before anything is stored. +// newShared builds the values for format f and the options of a call. Without marshalers, it builds those too. +// It panics if a second numeric format is accepted. +func newShared(f Format, config decodeConfig, marshalers *json.Marshalers) *shared { dc := newDecoder(f, config) dc.goType = reflect.TypeFor[time.Duration]() - s := &shared{unmarshalers: json.UnmarshalFromFunc(dc.decode)} - if config == (decodeConfig{}) { + if marshalers == nil { c := f.mustCodec() - s.marshalers = json.MarshalToFunc(func(enc *jsontext.Encoder, d time.Duration) error { + marshalers = json.MarshalToFunc(func(enc *jsontext.Encoder, d time.Duration) error { return encode(enc, c, d) }) - } else { - // Options change only how durations are read, so the marshalers are those of the format alone. - s.marshalers = sharedFor(f, nil).marshalers } - s.options = json.JoinOptions(json.WithMarshalers(s.marshalers), json.WithUnmarshalers(s.unmarshalers)) + unmarshalers := json.UnmarshalFromFunc(dc.decode) - // If another goroutine was faster, its values are the ones to share. - slot.CompareAndSwap(nil, s) - return slot.Load() + return &shared{ + marshalers: marshalers, + unmarshalers: unmarshalers, + options: json.JoinOptions(json.WithMarshalers(marshalers), json.WithUnmarshalers(unmarshalers)), + } } diff --git a/shared_test.go b/shared_test.go index aee1f2d..fe35f24 100644 --- a/shared_test.go +++ b/shared_test.go @@ -3,47 +3,40 @@ package jsonduration import ( "bytes" "encoding/json/v2" - "sync" "testing" "time" ) -// TestSameArgumentsShareOptions checks which calls get the same values: those that write the same format and read the -// same formats. It compares the unmarshalers, since they are pointers and the options are not comparable. -func TestSameArgumentsShareOptions(t *testing.T) { - base := Unmarshalers(ISO8601, AlsoAccept(Nano, Units)) - - same := map[string]*json.Unmarshalers{ - "again": Unmarshalers(ISO8601, AlsoAccept(Nano, Units)), - "other order": Unmarshalers(ISO8601, AlsoAccept(Units, Nano)), - "a format twice": Unmarshalers(ISO8601, AlsoAccept(Nano, Units, ISO8601, Nano)), - "two options": Unmarshalers(ISO8601, AlsoAccept(), AlsoAccept(Nano), AlsoAccept(Units)), - "a nil option": Unmarshalers(ISO8601, nil, AlsoAccept(Nano, Units)), - "the written one": Unmarshalers(ISO8601, AlsoAccept(ISO8601), AlsoAccept(Nano, Units)), - } - for name, got := range same { - if got != base { - t.Errorf("%s: got other unmarshalers, want the shared ones", name) +// TestFormatSharesOptions checks which calls get the same values: those that write the same format and have no +// option that changes anything. It compares the unmarshalers, since they are pointers and the options are not +// comparable. +func TestFormatSharesOptions(t *testing.T) { + for _, f := range Formats() { + if first, second := Marshalers(f), Marshalers(f); first != second { + t.Errorf("Marshalers(%s) differ between calls", f) } - } - different := map[string]*json.Unmarshalers{ - "other written format": Unmarshalers(Nano, AlsoAccept(ISO8601, Units)), - "no other formats": Unmarshalers(ISO8601), - "fewer formats": Unmarshalers(ISO8601, AlsoAccept(Nano)), - "other format": Unmarshalers(ISO8601, AlsoAccept(Sec, Units)), - "with days": Unmarshalers(ISO8601, AlsoAccept(Nano, Units), AcceptDays()), - } - for name, got := range different { - if got == base { - t.Errorf("%s: got the unmarshalers of ISO8601 with Nano and Units", name) + base := Unmarshalers(f) + + same := map[string]*json.Unmarshalers{ + "again": Unmarshalers(f), + "a nil option": Unmarshalers(f, nil), + "an empty option": Unmarshalers(f, AlsoAccept()), + } + for name, got := range same { + if got != base { + t.Errorf("%s, %s: got other unmarshalers, want the shared ones", f, name) + } } - } - for _, f := range Formats() { - first, second := Marshalers(f), Marshalers(f) - if first != second { - t.Errorf("Marshalers(%s) differ between calls", f) + different := map[string]*json.Unmarshalers{ + "another format": Unmarshalers(f, AlsoAccept(Units, ISO8601)), + "with days": Unmarshalers(f, AcceptDays()), + } + for name, got := range different { + if got == base { + t.Errorf("%s, %s: got the unmarshalers of the format alone", f, name) + } } } } @@ -61,9 +54,9 @@ func TestOptionsDoNotChangeMarshalers(t *testing.T) { } } -// TestWithDoesNotAllocate checks the promise in the documentation of With: asking again costs no allocation, -// with decoding options or without, so that decoding with options from a call allocates as little as with kept ones, -// which is not at all. +// TestWithDoesNotAllocate checks the promise in the documentation of With: without decoding options, asking again +// costs no allocation, so that decoding with options from a call allocates as little as with kept ones, which is not +// at all. func TestWithDoesNotAllocate(t *testing.T) { for _, f := range Formats() { in, err := json.Marshal(d90, With(f)) @@ -81,10 +74,7 @@ func TestWithDoesNotAllocate(t *testing.T) { {"Marshalers", 0, func() { _ = Marshalers(f) }}, {"Unmarshalers", 0, func() { _ = Unmarshalers(f) }}, {"With a nil option", 0, func() { _ = With(f, nil) }}, - {"With options", 0, func() { _ = With(f, AlsoAccept(Units, ISO8601), AcceptDays()) }}, - {"Unmarshalers with options", 0, func() { _ = Unmarshalers(f, AlsoAccept(Units, ISO8601), AcceptDays()) }}, {"Unmarshal", 0, func() { _ = json.Unmarshal(in, &d, With(f)) }}, - {"Unmarshal with options", 0, func() { _ = json.Unmarshal(in, &d, With(f, AlsoAccept(Units, ISO8601))) }}, } for _, tt := range tests { @@ -95,55 +85,6 @@ func TestWithDoesNotAllocate(t *testing.T) { } } -// TestInvalidArgumentsAreNotStored checks that a call that panics leaves nothing behind, -// so that the shared values cannot grow without bound, and that it panics again the next time. -func TestInvalidArgumentsAreNotStored(t *testing.T) { - count := func() int { - n := 0 - for i := range sharedByArgs { - for j := range sharedByArgs[i] { - for k := range sharedByArgs[i][j] { - if sharedByArgs[i][j][k].Load() != nil { - n++ - } - } - } - } - - return n - } - - // Build the valid part first: afterwards, an invalid call must not find it. - With(ISO8601) - before := count() - - invalid := map[string]func(){ - "unknown format": func() { With("bogus") }, - "empty format": func() { With("") }, - "unknown accepted": func() { With(ISO8601, AlsoAccept("bogus")) }, - "empty accepted": func() { With(ISO8601, AlsoAccept("")) }, - "two numeric": func() { With(Nano, AlsoAccept(Sec)) }, - "unknown after a second": func() { With(Nano, AlsoAccept(Sec, "bogus")) }, - } - - for name, fn := range invalid { - for range 2 { - func() { - defer func() { - if recover() == nil { - t.Errorf("%s: no panic", name) - } - }() - fn() - }() - } - } - - if after := count(); after != before { - t.Errorf("%d shared values after invalid calls, want %d as before", after, before) - } -} - // TestOptionValueIsReusable checks that an option used in several calls is not changed by the options after it. func TestOptionValueIsReusable(t *testing.T) { units := AlsoAccept(Units) @@ -166,26 +107,14 @@ func TestOptionValueIsReusable(t *testing.T) { if err := json.Unmarshal([]byte(inUnits), &d, onlyUnits); err != nil || d != d90 { t.Errorf("Unmarshal(%s) with units only = %v, %v, want %v", inUnits, d, err, d90) } -} - -// TestConcurrentFirstCalls only finds a data race when run with -race, as CI does. -// Every goroutine must get the same values, whichever built them. -func TestConcurrentFirstCalls(t *testing.T) { - // No other test asks for this set of arguments, so these calls are the first. - args := func() *json.Unmarshalers { return Unmarshalers(Micro, AlsoAccept(Units, ISO8601), AcceptDays()) } - - got := make([]*json.Unmarshalers, 16) - - var wg sync.WaitGroup - for i := range got { - wg.Go(func() { got[i] = args() }) - } - wg.Wait() + // The option keeps the formats it was given, also if the caller changes the slice afterwards. + formats := []Format{Units} + fromSlice := AlsoAccept(formats...) + formats[0] = ISO8601 - for i, u := range got { - if u != got[0] { - t.Errorf("goroutine %d got other unmarshalers than goroutine 0", i) - } + d = 0 + if err := json.Unmarshal([]byte(inUnits), &d, With(Sec, fromSlice)); err != nil || d != d90 { + t.Errorf("Unmarshal(%s) after the slice changed = %v, %v, want %v", inUnits, d, err, d90) } }