Optimize benchmarks; permit serialization #88

lavalamp · 2019-07-16T23:47:24Z

Prior to these changes:

BenchmarkDeducedSimple-12                                   3000            472665 ns/op          139180 B/op       1810 allocs/op
BenchmarkDeducedNested-12                                   1000           1370363 ns/op          401776 B/op       5146 allocs/op
BenchmarkDeducedNestedAcrossVersion-12                      1000           1435743 ns/op          434426 B/op       5527 allocs/op
BenchmarkLeafConflictAcrossVersion-12                       3000            492026 ns/op          147113 B/op       1848 allocs/op
BenchmarkMultipleApplierRecursiveRealConversion-12          1000           2585529 ns/op          805147 B/op       8457 allocs/op

After these changes:

BenchmarkDeducedSimple-12                                   5000            432763 ns/op          127173 B/op       1703 allocs/op
BenchmarkDeducedNested-12                                   1000           1298198 ns/op          356460 B/op       4678 allocs/op
BenchmarkDeducedNestedAcrossVersion-12                      1000           1359040 ns/op          383408 B/op       5022 allocs/op
BenchmarkLeafConflictAcrossVersion-12                       3000            437574 ns/op          134185 B/op       1783 allocs/op
BenchmarkMultipleApplierRecursiveRealConversion-12           500           2382065 ns/op          732700 B/op       7978 allocs/op

Mostly I was looking at allocations but this is a modest improvement for everything.

This will permit a straightforward serialization. The current map with string key form can't be deserialized in a single step.

k8s-ci-robot · 2019-07-16T23:47:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lavalamp

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [lavalamp]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

lavalamp · 2019-07-17T03:48:48Z

Antoine is OOO. I think this should be ready to go. I haven't tried this on the main repo but this is definitely not worse than what we have now unless the benchmarks are completely unrepresentative.

/assign @jennybuckley @kwiesmueller

apelisse · 2019-07-17T08:45:13Z

Benchmarks in this repo use significantly smaller sets than real-life sets. I doubt this is going to be equally good with real-life size objects :-/

lavalamp · 2019-07-17T15:58:59Z

Benchmark comparison:

benchmark                                              old ns/op     new ns/op     delta
BenchmarkDeducedSimple-12                              472665        432763        -8.44%
BenchmarkDeducedNested-12                              1370363       1298198       -5.27%
BenchmarkDeducedNestedAcrossVersion-12                 1435743       1359040       -5.34%
BenchmarkLeafConflictAcrossVersion-12                  492026        437574        -11.07%
BenchmarkMultipleApplierRecursiveRealConversion-12     2585529       2382065       -7.87%

benchmark                                              old allocs     new allocs     delta
BenchmarkDeducedSimple-12                              1810           1703           -5.91%
BenchmarkDeducedNested-12                              5146           4678           -9.09%
BenchmarkDeducedNestedAcrossVersion-12                 5527           5022           -9.14%
BenchmarkLeafConflictAcrossVersion-12                  1848           1783           -3.52%
BenchmarkMultipleApplierRecursiveRealConversion-12     8457           7978           -5.66%

benchmark                                              old bytes     new bytes     delta
BenchmarkDeducedSimple-12                              139180        127173        -8.63%
BenchmarkDeducedNested-12                              401776        356460        -11.28%
BenchmarkDeducedNestedAcrossVersion-12                 434426        383408        -11.74%
BenchmarkLeafConflictAcrossVersion-12                  147113        134185        -8.79%
BenchmarkMultipleApplierRecursiveRealConversion-12     805147        732700        -9.00%

lavalamp · 2019-07-17T15:59:30Z

I'll test in the main repo, then. :'(

apelisse · 2019-07-17T16:00:41Z

Wouldn't it be better to improve these tests? Not sure what is the best way to make them more real-life size.

lavalamp · 2019-07-17T16:50:39Z

The benchmark code here is running a bunch of test assertions, diluting the signal here. After making it not do that, the results are not changed much, maybe slightly worse. Time seems pretty noisy, but I'm looking at allocations.

I started to make the items in the benchmarks bigger but that is a harder task than expected, plus I seem to have hit a bug where field names are not handled right, e.g. letter followed by number has the number stripped.

benchmark                                              old ns/op     new ns/op     delta
BenchmarkDeducedSimple-12                              453044        458392        +1.18%
BenchmarkDeducedNested-12                              1382705       1266794       -8.38%
BenchmarkDeducedNestedAcrossVersion-12                 1501691       1388808       -7.52%
BenchmarkLeafConflictAcrossVersion-12                  465940        454865        -2.38%
BenchmarkMultipleApplierRecursiveRealConversion-12     2498657       2328747       -6.80%

benchmark                                              old allocs     new allocs     delta
BenchmarkDeducedSimple-12                              1810           1703           -5.91%
BenchmarkDeducedNested-12                              5146           4678           -9.09%
BenchmarkDeducedNestedAcrossVersion-12                 5527           5022           -9.14%
BenchmarkLeafConflictAcrossVersion-12                  1848           1783           -3.52%
BenchmarkMultipleApplierRecursiveRealConversion-12     8149           7694           -5.58%

benchmark                                              old bytes     new bytes     delta
BenchmarkDeducedSimple-12                              139179        127172        -8.63%
BenchmarkDeducedNested-12                              401784        356427        -11.29%
BenchmarkDeducedNestedAcrossVersion-12                 434410        383419        -11.74%
BenchmarkLeafConflictAcrossVersion-12                  147112        134184        -8.79%
BenchmarkMultipleApplierRecursiveRealConversion-12     784542        711478        -9.31%

lavalamp · 2019-07-18T05:47:06Z

I've added a microbenchmark for set.

benchmark                                  old ns/op     new ns/op     delta
BenchmarkFieldSet/insert-10-12             11498         6663          -42.05%
BenchmarkFieldSet/has-10-12                910           748           -17.80%
BenchmarkFieldSet/union-10-12              4195          4132          -1.50%
BenchmarkFieldSet/intersection-10-12       1903          2790          +46.61%
BenchmarkFieldSet/difference-10-12         1893          3152          +66.51%
BenchmarkFieldSet/insert-20-12             58147         26062         -55.18%
BenchmarkFieldSet/has-20-12                1710          1037          -39.36%
BenchmarkFieldSet/union-20-12              41174         15651         -61.99%
BenchmarkFieldSet/intersection-20-12       8791          7893          -10.21%
BenchmarkFieldSet/difference-20-12         28758         12580         -56.26%
BenchmarkFieldSet/insert-50-12             186330        89396         -52.02%
BenchmarkFieldSet/has-50-12                1978          1354          -31.55%
BenchmarkFieldSet/union-50-12              131180        40162         -69.38%
BenchmarkFieldSet/intersection-50-12       21577         17659         -18.16%
BenchmarkFieldSet/difference-50-12         93593         30367         -67.55%
BenchmarkFieldSet/insert-100-12            676984        278113        -58.92%
BenchmarkFieldSet/has-100-12               2794          2016          -27.85%
BenchmarkFieldSet/union-100-12             262925        94525         -64.05%
BenchmarkFieldSet/intersection-100-12      31648         43703         +38.09%
BenchmarkFieldSet/difference-100-12        162427        74614         -54.06%
BenchmarkFieldSet/insert-500-12            3433305       1595039       -53.54%
BenchmarkFieldSet/has-500-12               3517          2639          -24.96%
BenchmarkFieldSet/union-500-12             1247269       392640        -68.52%
BenchmarkFieldSet/intersection-500-12      214700        183495        -14.53%
BenchmarkFieldSet/difference-500-12        873070        294497        -66.27%
BenchmarkFieldSet/insert-1000-12           8943251       3711783       -58.50%
BenchmarkFieldSet/has-1000-12              4074          2888          -29.11%
BenchmarkFieldSet/union-1000-12            2432131       714658        -70.62%
BenchmarkFieldSet/intersection-1000-12     461434        342822        -25.71%
BenchmarkFieldSet/difference-1000-12       1699348       586396        -65.49%

benchmark                                  old allocs     new allocs     delta
BenchmarkFieldSet/insert-10-12             50             31             -38.00%
BenchmarkFieldSet/has-10-12                4              5              +25.00%
BenchmarkFieldSet/union-10-12              6              21             +250.00%
BenchmarkFieldSet/intersection-10-12       5              19             +280.00%
BenchmarkFieldSet/difference-10-12         4              19             +375.00%
BenchmarkFieldSet/insert-20-12             231            134            -41.99%
BenchmarkFieldSet/has-20-12                8              6              -25.00%
BenchmarkFieldSet/union-20-12              127            85             -33.07%
BenchmarkFieldSet/intersection-20-12       45             61             +35.56%
BenchmarkFieldSet/difference-20-12         102            74             -27.45%
BenchmarkFieldSet/insert-50-12             743            446            -39.97%
BenchmarkFieldSet/has-50-12                9              7              -22.22%
BenchmarkFieldSet/union-50-12              416            196            -52.88%
BenchmarkFieldSet/intersection-50-12       93             126            +35.48%
BenchmarkFieldSet/difference-50-12         302            163            -46.03%
BenchmarkFieldSet/insert-100-12            2462           1338           -45.65%
BenchmarkFieldSet/has-100-12               11             9              -18.18%
BenchmarkFieldSet/union-100-12             1241           435            -64.95%
BenchmarkFieldSet/intersection-100-12      205            305            +48.78%
BenchmarkFieldSet/difference-100-12        818            388            -52.57%
BenchmarkFieldSet/insert-500-12            13424          8013           -40.31%
BenchmarkFieldSet/has-500-12               13             12             -7.69%
BenchmarkFieldSet/union-500-12             6027           2341           -61.16%
BenchmarkFieldSet/intersection-500-12      1215           1604           +32.02%
BenchmarkFieldSet/difference-500-12        4240           1998           -52.88%
BenchmarkFieldSet/insert-1000-12           29646          17782          -40.02%
BenchmarkFieldSet/has-1000-12              14             13             -7.14%
BenchmarkFieldSet/union-1000-12            11738          4413           -62.40%
BenchmarkFieldSet/intersection-1000-12     2198           2848           +29.57%
BenchmarkFieldSet/difference-1000-12       8040           3756           -53.28%

As you can see, it's a massive improvement with a few exceptions, mostly intersection.

apelisse

I'm surprised with the very high variance (15-25%) across consecutive runs. I looked carefully and I don't see any obvious reasons for it. I'll investigate a little bit more. (I'm a little surprised by some of your old/new benchmark being sometimes better and sometimes worse in a non-linear way).

apelisse · 2019-07-18T11:57:19Z

merge/deduced_test.go

@@ -598,7 +598,13 @@ func BenchmarkDeducedSimple(b *testing.B) {
 		},
 	}

+	// Make sure this passes...
+	if err := test.Test(typed.DeducedParseableType); err != nil {


You kept the same test below in the loop, is that on purpose? (same for other benches below)

ah that explains why the numbers didn't change much, haha

benchmark old ns/op new ns/op delta BenchmarkDeducedSimple-12 422922 415645 -1.72% BenchmarkDeducedNested-12 1252984 1179542 -5.86% BenchmarkDeducedNestedAcrossVersion-12 1395454 1293546 -7.30% BenchmarkLeafConflictAcrossVersion-12 412367 410950 -0.34% BenchmarkMultipleApplierRecursiveRealConversion-12 2481784 2413984 -2.73% benchmark old allocs new allocs delta BenchmarkDeducedSimple-12 1634 1533 -6.18% BenchmarkDeducedNested-12 4757 4289 -9.84% BenchmarkDeducedNestedAcrossVersion-12 5138 4633 -9.83% BenchmarkLeafConflictAcrossVersion-12 1672 1613 -3.53% BenchmarkMultipleApplierRecursiveRealConversion-12 8154 7696 -5.62% benchmark old bytes new bytes delta BenchmarkDeducedSimple-12 126676 114318 -9.76% BenchmarkDeducedNested-12 368885 323195 -12.39% BenchmarkDeducedNestedAcrossVersion-12 401554 350212 -12.79% BenchmarkLeafConflictAcrossVersion-12 134616 121344 -9.86% BenchmarkMultipleApplierRecursiveRealConversion-12 784880 711578 -9.34%

After fixing these (not pushed yet).

apelisse · 2019-07-18T11:59:55Z

fieldpath/set_test.go

+		randOperand := func() *Set { return operands[rand.Intn(len(operands))] }
+		b.Run(fmt.Sprintf("union-%v", here.size), func(b *testing.B) {
+			b.ResetTimer()
+			b.ReportAllocs()


I'm curious if these two are needed

apelisse · 2019-07-18T12:20:47Z

I just had this in two consecutive runs:

BenchmarkFieldSet/difference-50-8                 11911         26110         +119.21%

lavalamp · 2019-07-18T17:07:16Z

I tweaked the benchmarks a bit. I did three runs of old and new, which were all very consistent with each other. Here's a random old / new comparison:

benchmark                                  old ns/op     new ns/op     delta
BenchmarkFieldSet/insert-20-12             54252         24868         -54.16%
BenchmarkFieldSet/has-20-12                1597          1306          -18.22%
BenchmarkFieldSet/union-20-12              36040         17464         -51.54%
BenchmarkFieldSet/intersection-20-12       7631          8752          +14.69%
BenchmarkFieldSet/difference-20-12         25865         13741         -46.87%
BenchmarkFieldSet/insert-50-12             149605        90214         -39.70%
BenchmarkFieldSet/has-50-12                1776          1438          -19.03%
BenchmarkFieldSet/union-50-12              95960         37428         -61.00%
BenchmarkFieldSet/intersection-50-12       16027         15331         -4.34%
BenchmarkFieldSet/difference-50-12         67187         28200         -58.03%
BenchmarkFieldSet/insert-100-12            498853        233598        -53.17%
BenchmarkFieldSet/has-100-12               2357          1864          -20.92%
BenchmarkFieldSet/union-100-12             246561        73341         -70.25%
BenchmarkFieldSet/intersection-100-12      34281         33725         -1.62%
BenchmarkFieldSet/difference-100-12        170985        63803         -62.69%
BenchmarkFieldSet/insert-500-12            2797068       1407020       -49.70%
BenchmarkFieldSet/has-500-12               2607          2375          -8.90%
BenchmarkFieldSet/union-500-12             1095431       383595        -64.98%
BenchmarkFieldSet/intersection-500-12      219792        205134        -6.67%
BenchmarkFieldSet/difference-500-12        778033        352579        -54.68%
BenchmarkFieldSet/insert-1000-12           5288591       2782317       -47.39%
BenchmarkFieldSet/has-1000-12              2800          2298          -17.93%
BenchmarkFieldSet/union-1000-12            2039674       692459        -66.05%
BenchmarkFieldSet/intersection-1000-12     436493        352916        -19.15%
BenchmarkFieldSet/difference-1000-12       1457956       738868        -49.32%

benchmark                                  old allocs     new allocs     delta
BenchmarkFieldSet/insert-20-12             227            130            -42.73%
BenchmarkFieldSet/has-20-12                7              6              -14.29%
BenchmarkFieldSet/union-20-12              129            84             -34.88%
BenchmarkFieldSet/intersection-20-12       43             60             +39.53%
BenchmarkFieldSet/difference-20-12         105            73             -30.48%
BenchmarkFieldSet/insert-50-12             729            435            -40.33%
BenchmarkFieldSet/has-50-12                8              7              -12.50%
BenchmarkFieldSet/union-50-12              424            190            -55.19%
BenchmarkFieldSet/intersection-50-12       89             117            +31.46%
BenchmarkFieldSet/difference-50-12         310            157            -49.35%
BenchmarkFieldSet/insert-100-12            2413           1309           -45.75%
BenchmarkFieldSet/has-100-12               10             9              -10.00%
BenchmarkFieldSet/union-100-12             1203           401            -66.67%
BenchmarkFieldSet/intersection-100-12      169            268            +58.58%
BenchmarkFieldSet/difference-100-12        801            357            -55.43%
BenchmarkFieldSet/insert-500-12            13179          7754           -41.16%
BenchmarkFieldSet/has-500-12               12             12             +0.00%
BenchmarkFieldSet/union-500-12             5862           2198           -62.50%
BenchmarkFieldSet/intersection-500-12      1013           1469           +45.01%
BenchmarkFieldSet/difference-500-12        4169           1886           -54.76%
BenchmarkFieldSet/insert-1000-12           29000          17208          -40.66%
BenchmarkFieldSet/has-1000-12              13             13             +0.00%
BenchmarkFieldSet/union-1000-12            11506          4118           -64.21%
BenchmarkFieldSet/intersection-1000-12     1841           2585           +40.41%
BenchmarkFieldSet/difference-1000-12       7994           3492           -56.32%

Even though intersection does more allocations, it's still somehow faster.

lavalamp · 2019-07-18T19:04:17Z

I made another change to permit it to cache the order for PathElement.Keys. It was apparently a huge deal.

benchmark                                  old ns/op     new ns/op     delta
BenchmarkFieldSet/insert-20-12             54252         19315         -64.40%
BenchmarkFieldSet/has-20-12                1597          704           -55.92%
BenchmarkFieldSet/union-20-12              36040         11495         -68.10%
BenchmarkFieldSet/intersection-20-12       7631          5153          -32.47%
BenchmarkFieldSet/difference-20-12         25865         8763          -66.12%
BenchmarkFieldSet/insert-50-12             149605        62701         -58.09%
BenchmarkFieldSet/has-50-12                1776          873           -50.84%
BenchmarkFieldSet/union-50-12              95960         29569         -69.19%
BenchmarkFieldSet/intersection-50-12       16027         10213         -36.28%
BenchmarkFieldSet/difference-50-12         67187         20901         -68.89%
BenchmarkFieldSet/insert-100-12            498853        165516        -66.82%
BenchmarkFieldSet/has-100-12               2357          1287          -45.40%
BenchmarkFieldSet/union-100-12             246561        62777         -74.54%
BenchmarkFieldSet/intersection-100-12      34281         20617         -39.86%
BenchmarkFieldSet/difference-100-12        170985        46939         -72.55%
BenchmarkFieldSet/insert-500-12            2797068       770229        -72.46%
BenchmarkFieldSet/has-500-12               2607          1459          -44.04%
BenchmarkFieldSet/union-500-12             1095431       283355        -74.13%
BenchmarkFieldSet/intersection-500-12      219792        126864        -42.28%
BenchmarkFieldSet/difference-500-12        778033        260655        -66.50%
BenchmarkFieldSet/insert-1000-12           5288591       2120121       -59.91%
BenchmarkFieldSet/has-1000-12              2800          1360          -51.43%
BenchmarkFieldSet/union-1000-12            2039674       540864        -73.48%
BenchmarkFieldSet/intersection-1000-12     436493        213266        -51.14%
BenchmarkFieldSet/difference-1000-12       1457956       394617        -72.93%

benchmark                                  old allocs     new allocs     delta
BenchmarkFieldSet/insert-20-12             227            78             -65.64%
BenchmarkFieldSet/has-20-12                7              2              -71.43%
BenchmarkFieldSet/union-20-12              129            54             -58.14%
BenchmarkFieldSet/intersection-20-12       43             29             -32.56%
BenchmarkFieldSet/difference-20-12         105            43             -59.05%
BenchmarkFieldSet/insert-50-12             729            247            -66.12%
BenchmarkFieldSet/has-50-12                8              2              -75.00%
BenchmarkFieldSet/union-50-12              424            133            -68.63%
BenchmarkFieldSet/intersection-50-12       89             61             -31.46%
BenchmarkFieldSet/difference-50-12         310            101            -67.42%
BenchmarkFieldSet/insert-100-12            2413           813            -66.31%
BenchmarkFieldSet/has-100-12               10             3              -70.00%
BenchmarkFieldSet/union-100-12             1203           257            -78.64%
BenchmarkFieldSet/intersection-100-12      169            124            -26.63%
BenchmarkFieldSet/difference-100-12        801            214            -73.28%
BenchmarkFieldSet/insert-500-12            13179          4141           -68.58%
BenchmarkFieldSet/has-500-12               12             3              -75.00%
BenchmarkFieldSet/union-500-12             5862           1468           -74.96%
BenchmarkFieldSet/intersection-500-12      1013           739            -27.05%
BenchmarkFieldSet/difference-500-12        4169           1162           -72.13%
BenchmarkFieldSet/insert-1000-12           29000          8981           -69.03%
BenchmarkFieldSet/has-1000-12              13             3              -76.92%
BenchmarkFieldSet/union-1000-12            11506          2842           -75.30%
BenchmarkFieldSet/intersection-1000-12     1841           1330           -27.76%
BenchmarkFieldSet/difference-1000-12       7994           2238           -72.00%

lavalamp · 2019-07-18T19:13:55Z

I also looked at a mem & cpu profile while running the benchmarks and I don't think there's anything left that's easy to improve now. The latest change improved the original benchmarks, too:

benchmark                                              old ns/op     new ns/op     delta
BenchmarkDeducedSimple-12                              422922        404477        -4.36%
BenchmarkDeducedNested-12                              1252984       1054765       -15.82%
BenchmarkDeducedNestedAcrossVersion-12                 1395454       1129654       -19.05%
BenchmarkLeafConflictAcrossVersion-12                  412367        393057        -4.68%
BenchmarkMultipleApplierRecursiveRealConversion-12     2481784       2242044       -9.66%

benchmark                                              old allocs     new allocs     delta
BenchmarkDeducedSimple-12                              1634           1525           -6.67%
BenchmarkDeducedNested-12                              4757           4180           -12.13%
BenchmarkDeducedNestedAcrossVersion-12                 5138           4524           -11.95%
BenchmarkLeafConflictAcrossVersion-12                  1672           1605           -4.01%
BenchmarkMultipleApplierRecursiveRealConversion-12     8154           7692           -5.67%

I'd like to squash and merge this, can someone review it first?

… on all benchmarks; needs more testing of the lexical operator

This is technically a breaking change, but most people won't construct this directly so it should be low-impact.

lavalamp · 2019-07-18T20:37:01Z

I squashed this into some sensible commits. @jennybuckley can you LGTM?

apelisse

/lgtm

apelisse · 2019-07-19T07:42:35Z

fieldpath/element.go

+	if s.members[loc].Equals(pe) {
+		return
+	}
+	n := len(s.members) - 1


Mostly similar but maybe better: https://github.com/golang/go/wiki/SliceTricks#insert

Yeah I meant to switch this to the thing they have listed as the "copy" variant; the other variant does an unnecessary allocation.

apelisse · 2019-07-19T08:39:15Z

The final benchmarks are a little underwhelming, compared to the improvements to set operations.

lavalamp · 2019-07-19T15:44:31Z

The final benchmarks are a little underwhelming, compared to the improvements to set operations.

I think the string / yaml conversion is probably dominating the performance in those benchmarks.

lavalamp · 2019-07-19T22:27:26Z

I managed to run this in the main repo, will send an update PR there I guess if I can figure out the dependency stuff, but here's the results:

$ benchcmp orig.txt new.txt
benchmark                       old ns/op     new ns/op     delta
BenchmarkApplyNewObject-12      2230683       2129184       -4.55%
BenchmarkUpdateNewObject-12     2624417       2379121       -9.35%
BenchmarkRepeatedUpdate-12      1031653       981264        -4.88%
BenchmarkSetToFields-12         24778         23686         -4.41%
BenchmarkFieldsToSet-12         33876         17971         -46.95%
BenchmarkYAMLToTyped-12         68377         64312         -5.94%

benchmark                       old allocs     new allocs     delta
BenchmarkApplyNewObject-12      8632           7168           -16.96%
BenchmarkUpdateNewObject-12     11210          8396           -25.10%
BenchmarkRepeatedUpdate-12      3057           2887           -5.56%
BenchmarkSetToFields-12         124            124            +0.00%
BenchmarkFieldsToSet-12         147            93             -36.73%
BenchmarkYAMLToTyped-12         150            152            +1.33%

benchmark                       old bytes     new bytes     delta
BenchmarkApplyNewObject-12      555847        500704        -9.92%
BenchmarkUpdateNewObject-12     659671        560262        -15.07%
BenchmarkRepeatedUpdate-12      255150        221120        -13.34%
BenchmarkSetToFields-12         8974          8369          -6.74%
BenchmarkFieldsToSet-12         12897         4688          -63.65%
BenchmarkYAMLToTyped-12         27625         27384         -0.87%

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jul 16, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 16, 2019

k8s-ci-robot requested review from apelisse and jennybuckley July 16, 2019 23:47

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jul 16, 2019

lavalamp changed the title ~~WIP: Optimize benchmarks~~ WIP: Optimize benchmarks; permit serialization Jul 17, 2019

lavalamp changed the title ~~WIP: Optimize benchmarks; permit serialization~~ Optimize benchmarks; permit serialization Jul 17, 2019

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 17, 2019

k8s-ci-robot assigned jennybuckley and kwiesmueller Jul 17, 2019

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 17, 2019

lavalamp force-pushed the optimize branch from a60e2ff to 13e34bf Compare July 17, 2019 16:52

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 17, 2019

lavalamp force-pushed the optimize branch from b637eb8 to 53624f1 Compare July 18, 2019 05:39

apelisse reviewed Jul 18, 2019

View reviewed changes

lavalamp added 4 commits July 18, 2019 13:33

don't use maps for PathElementSet; saves a couple hundred allocations…

f33c13e

… on all benchmarks; needs more testing of the lexical operator

In PathElement, replace []Field with *Map for Key

dd13317

This is technically a breaking change, but most people won't construct this directly so it should be low-impact.

Benchmarks shouldn't include test code

8b1ef26

Add a micro benchmark for fieldpath.

03f010c

lavalamp force-pushed the optimize branch from b8f4c05 to 03f010c Compare July 18, 2019 20:35

apelisse reviewed Jul 19, 2019

View reviewed changes

k8s-ci-robot assigned apelisse Jul 19, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 19, 2019

k8s-ci-robot merged commit f7547e8 into kubernetes-sigs:master Jul 19, 2019

Optimize benchmarks; permit serialization #88

Optimize benchmarks; permit serialization #88

Uh oh!

Conversation

lavalamp commented Jul 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Jul 16, 2019

Uh oh!

lavalamp commented Jul 17, 2019

Uh oh!

apelisse commented Jul 17, 2019

Uh oh!

lavalamp commented Jul 17, 2019

Uh oh!

lavalamp commented Jul 17, 2019

Uh oh!

apelisse commented Jul 17, 2019

Uh oh!

lavalamp commented Jul 17, 2019

Uh oh!

lavalamp commented Jul 18, 2019

Uh oh!

apelisse left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

apelisse Jul 18, 2019

Choose a reason for hiding this comment

Uh oh!

lavalamp Jul 18, 2019

Choose a reason for hiding this comment

Uh oh!

lavalamp Jul 18, 2019

Choose a reason for hiding this comment

Uh oh!

apelisse Jul 18, 2019

Choose a reason for hiding this comment

Uh oh!

apelisse commented Jul 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lavalamp commented Jul 18, 2019

Uh oh!

lavalamp commented Jul 18, 2019

Uh oh!

lavalamp commented Jul 18, 2019

Uh oh!

lavalamp commented Jul 18, 2019

Uh oh!

apelisse left a comment

Choose a reason for hiding this comment

Uh oh!

apelisse Jul 19, 2019

Choose a reason for hiding this comment

Uh oh!

lavalamp Jul 19, 2019

Choose a reason for hiding this comment

Uh oh!

lavalamp Jul 19, 2019

Choose a reason for hiding this comment

Uh oh!

apelisse commented Jul 19, 2019

Uh oh!

lavalamp commented Jul 19, 2019

Uh oh!

lavalamp commented Jul 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

lavalamp commented Jul 16, 2019 •

edited

Loading

apelisse left a comment •

edited

Loading

apelisse commented Jul 18, 2019 •

edited

Loading

lavalamp commented Jul 19, 2019 •

edited

Loading