Skip to content

Commit

Permalink
validateJSONMemberNames: Skip Unmarshal when validation disabled (#47)
Browse files Browse the repository at this point in the history
* validateJSONMemberNames: Skip Unmarshal when validation disabled

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%

* remove commented out validateJSONMemberNames

This was not intended to be committed.
  • Loading branch information
evanj authored Nov 6, 2023
1 parent 76a22db commit e2140a3
Show file tree
Hide file tree
Showing 3 changed files with 7 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
5 changes: 5 additions & 0 deletions member_names.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ 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)
Expand Down
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 e2140a3

Please sign in to comment.