Skip to content

Commit

Permalink
validateJSONMemberNames: Skip Unmarshal when validation disabled
Browse files Browse the repository at this point in the history
This function was calling json.Unmarshal even if validation was
disabled. This changes it to do nothing instead. This is a large
performance win for large documents.

Change the benchmarks to report memory allocation by default.

The benchstat output comparing the before/after for this change shows
that this reduces CPU time by 20% and bytes allocated by 30%.

                                                  │  before.txt  │              after.txt              │
                                                  │    sec/op    │   sec/op     vs base                │
Marshal/ArticleSimple-10                             4.054µ ± 2%   4.012µ ± 2%        ~ (p=0.218 n=10)
Marshal/ArticleSimpleWithToplevelMeta-10             5.135µ ± 2%   5.112µ ± 3%        ~ (p=1.000 n=10)
Marshal/ArticleComplex-10                            24.23µ ± 2%   23.81µ ± 3%        ~ (p=0.280 n=10)
Marshal/ArticleComplexDisableNameValidation-10       18.05µ ± 4%   11.90µ ± 5%  -34.09% (p=0.000 n=10)
Marshal/ArticlesComplex-10                           134.1µ ± 3%   133.0µ ± 2%        ~ (p=0.190 n=10)
Marshal/ArticlesComplexDisableNameValidation-10     102.99µ ± 3%   68.48µ ± 2%  -33.51% (p=0.000 n=10)
Unmarshal/ArticleSimple-10                           4.930µ ± 2%   4.952µ ± 2%        ~ (p=0.869 n=10)
Unmarshal/ArticleSimpleWithToplevelMeta-10           6.095µ ± 2%   6.028µ ± 3%        ~ (p=0.631 n=10)
Unmarshal/ArticleComplex-10                          35.42µ ± 2%   34.87µ ± 2%   -1.55% (p=0.035 n=10)
Unmarshal/ArticleComplexDisableNameValidation-10     29.66µ ± 2%   22.82µ ± 1%  -23.06% (p=0.000 n=10)
Unmarshal/ArticlesComplex-10                         181.4µ ± 2%   181.7µ ± 2%        ~ (p=0.684 n=10)
Unmarshal/ArticlesComplexDisableNameValidation-10    155.3µ ± 2%   119.7µ ± 2%  -22.95% (p=0.000 n=10)
geomean                                              26.35µ        23.43µ       -11.08%

                                                  │  before.txt   │               after.txt                │
                                                  │     B/op      │     B/op      vs base                  │
Marshal/ArticleSimple-10                             3.309Ki ± 0%   3.309Ki ± 0%        ~ (p=1.000 n=10) ¹
Marshal/ArticleSimpleWithToplevelMeta-10             3.872Ki ± 0%   3.872Ki ± 0%        ~ (p=1.000 n=10) ¹
Marshal/ArticleComplex-10                            16.66Ki ± 0%   16.67Ki ± 0%        ~ (p=0.370 n=10)
Marshal/ArticleComplexDisableNameValidation-10       16.66Ki ± 0%   11.02Ki ± 0%  -33.87% (p=0.000 n=10)
Marshal/ArticlesComplex-10                           87.76Ki ± 0%   87.76Ki ± 0%        ~ (p=0.382 n=10)
Marshal/ArticlesComplexDisableNameValidation-10      87.73Ki ± 0%   58.29Ki ± 0%  -33.56% (p=0.000 n=10)
Unmarshal/ArticleSimple-10                           3.063Ki ± 0%   3.063Ki ± 0%        ~ (p=1.000 n=10) ¹
Unmarshal/ArticleSimpleWithToplevelMeta-10           3.798Ki ± 0%   3.798Ki ± 0%        ~ (p=1.000 n=10) ¹
Unmarshal/ArticleComplex-10                          14.79Ki ± 0%   14.79Ki ± 0%        ~ (p=1.000 n=10) ¹
Unmarshal/ArticleComplexDisableNameValidation-10    14.779Ki ± 0%   8.659Ki ± 0%  -41.41% (p=0.000 n=10)
Unmarshal/ArticlesComplex-10                         69.25Ki ± 0%   69.25Ki ± 0%        ~ (p=0.200 n=10)
Unmarshal/ArticlesComplexDisableNameValidation-10    69.22Ki ± 0%   39.84Ki ± 0%  -42.45% (p=0.000 n=10)
geomean                                              16.23Ki        13.84Ki       -14.71%
¹ all samples are equal

                                                  │ before.txt  │               after.txt               │
                                                  │  allocs/op  │  allocs/op   vs base                  │
Marshal/ArticleSimple-10                             48.00 ± 0%    48.00 ± 0%        ~ (p=1.000 n=10) ¹
Marshal/ArticleSimpleWithToplevelMeta-10             60.00 ± 0%    60.00 ± 0%        ~ (p=1.000 n=10) ¹
Marshal/ArticleComplex-10                            257.0 ± 0%    257.0 ± 0%        ~ (p=1.000 n=10) ¹
Marshal/ArticleComplexDisableNameValidation-10       257.0 ± 0%    164.0 ± 0%  -36.19% (p=0.000 n=10)
Marshal/ArticlesComplex-10                          1.366k ± 0%   1.366k ± 0%        ~ (p=1.000 n=10) ¹
Marshal/ArticlesComplexDisableNameValidation-10     1366.0 ± 0%    869.0 ± 0%  -36.38% (p=0.000 n=10)
Unmarshal/ArticleSimple-10                           60.00 ± 0%    60.00 ± 0%        ~ (p=1.000 n=10) ¹
Unmarshal/ArticleSimpleWithToplevelMeta-10           73.00 ± 0%    73.00 ± 0%        ~ (p=1.000 n=10) ¹
Unmarshal/ArticleComplex-10                          275.0 ± 0%    275.0 ± 0%        ~ (p=1.000 n=10) ¹
Unmarshal/ArticleComplexDisableNameValidation-10     275.0 ± 0%    173.0 ± 0%  -37.09% (p=0.000 n=10)
Unmarshal/ArticlesComplex-10                        1.285k ± 0%   1.285k ± 0%        ~ (p=1.000 n=10) ¹
Unmarshal/ArticlesComplexDisableNameValidation-10   1285.0 ± 0%    788.0 ± 0%  -38.68% (p=0.000 n=10)
geomean                                              275.9         236.4       -14.32%
  • Loading branch information
evanj committed Oct 30, 2023
1 parent 76a22db commit 7cb35c2
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 0 deletions.
1 change: 1 addition & 0 deletions marshal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -817,6 +817,7 @@ func BenchmarkMarshal(b *testing.B) {
for _, bm := range benchmarks {
bm := bm
b.Run(bm.name, func(b *testing.B) {
b.ReportAllocs()
for n := 0; n < b.N; n++ {
_, _ = Marshal(bm.given, bm.opts...)
}
Expand Down
20 changes: 20 additions & 0 deletions member_names.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,29 @@ func validateMapMemberNames(m map[string]any, mode MemberNameValidationMode) err
}

func validateJSONMemberNames(b []byte, mode MemberNameValidationMode) error {
// do not unmarshal if validation is disabled
if mode == DisableValidation {
return nil
}

var m map[string]any
if err := json.Unmarshal(b, &m); err != nil {
return fmt.Errorf("unexpected unmarshal failure: %w", err)
}
return validateMapMemberNames(m, mode)
}

// func validateJSONMemberNames2(b []byte, mode MemberNameValidationMode) error {
// // do not unmarshal if validation is disabled
// if mode == DisableValidation {
// return nil
// }

// decoder := json.NewDecoder(bytes.NewReader(b))
// decoder.Token()
// var m map[string]any
// if err := json.Unmarshal(b, &m); err != nil {
// return fmt.Errorf("unexpected unmarshal failure: %w", err)
// }
// return validateMapMemberNames(m, mode)
// }
1 change: 1 addition & 0 deletions unmarshal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,7 @@ func BenchmarkUnmarshal(b *testing.B) {
bm := bm
data := []byte(bm.data)
b.Run(bm.name, func(b *testing.B) {
b.ReportAllocs()
for n := 0; n < b.N; n++ {
_ = Unmarshal(data, &bm.target, bm.opts...)
}
Expand Down

0 comments on commit 7cb35c2

Please sign in to comment.