Skip to content

Commit 98b8351

Browse files
authored
Backport: Track sizes of comprehension results (#901) (#902)
1 parent cfefae0 commit 98b8351

File tree

3 files changed

+132
-0
lines changed

3 files changed

+132
-0
lines changed

checker/cost.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,9 @@ func (c *coster) costComprehension(e *exprpb.Expr) CostEstimate {
520520
c.iterRanges.pop(comp.GetIterVar())
521521
sum = sum.Add(c.cost(comp.Result))
522522
rangeCnt := c.sizeEstimate(c.newAstNode(comp.GetIterRange()))
523+
524+
c.computedSizes[e.GetId()] = rangeCnt
525+
523526
rangeCost := rangeCnt.MultiplyByCost(stepCost.Add(loopCost))
524527
sum = sum.Add(rangeCost)
525528

checker/cost_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,71 @@ func TestCost(t *testing.T) {
502502
},
503503
wanted: CostEstimate{Min: 3, Max: 3},
504504
},
505+
{
506+
name: ".filter list literal",
507+
expr: `[1,2,3,4,5].filter(x, x % 2 == 0)`,
508+
wanted: CostEstimate{Min: 41, Max: 101},
509+
},
510+
{
511+
name: ".map list literal",
512+
expr: `[1,2,3,4,5].map(x, x)`,
513+
wanted: CostEstimate{Min: 86, Max: 86},
514+
},
515+
{
516+
name: ".map.filter list literal",
517+
expr: `[1,2,3,4,5].map(x, x).filter(x, x % 2 == 0)`,
518+
wanted: CostEstimate{Min: 117, Max: 177},
519+
},
520+
{
521+
name: ".map.exists list literal",
522+
expr: `[1,2,3,4,5].map(x, x).exists(x, x == 5) == true`,
523+
wanted: CostEstimate{Min: 108, Max: 118},
524+
},
525+
{
526+
name: ".map.map list literal",
527+
expr: `[1,2,3,4,5].map(x, x).map(x, x)`,
528+
wanted: CostEstimate{Min: 162, Max: 162},
529+
},
530+
{
531+
name: ".map list literal selection",
532+
expr: `[1,2,3,4,5].map(x, x)[4]`,
533+
wanted: CostEstimate{Min: 87, Max: 87},
534+
},
535+
{
536+
name: "nested array selection",
537+
expr: `[[1,2],[1,2],[1,2],[1,2],[1,2]][4]`,
538+
wanted: CostEstimate{Min: 61, Max: 61},
539+
},
540+
{
541+
name: "nested array selection",
542+
expr: `{'a': [1,2], 'b': [1,2], 'c': [1,2], 'd': [1,2], 'e': [1,2]}.b`,
543+
wanted: CostEstimate{Min: 81, Max: 81},
544+
},
545+
{
546+
// Estimated cost does not track the sizes of nested aggregate types
547+
// (lists, maps, ...) and so assumes a worst case cost when an
548+
// expression applies a comprehension to a nested aggregated type,
549+
// even if the size information is available.
550+
// TODO: This should be fixed.
551+
name: "comprehension on nested list",
552+
expr: `[1,2,3,4,5].map(x, [x, x]).all(y, y.all(y, y == 1))`,
553+
wanted: CostEstimate{Min: 157, Max: 18446744073709551615},
554+
},
555+
{
556+
// Make sure we're accounting for not just the iteration range size,
557+
// but also the overall comprehension size. The chained map calls
558+
// will treat the result of one map as the iteration range of the other,
559+
// so they're planned in reverse; however, the `+` should verify that
560+
// the comprehension result has a size.
561+
name: "comprehension size",
562+
expr: `[1,2,3,4,5].map(x, x).map(x, x) + [1]`,
563+
wanted: CostEstimate{Min: 173, Max: 173},
564+
},
565+
{
566+
name: "nested comprehension",
567+
expr: `[1,2,3].all(i, i in [1,2,3].map(j, j + j))`,
568+
wanted: CostEstimate{Min: 20, Max: 230},
569+
},
505570
}
506571

507572
for _, tc := range cases {

interpreter/runtimecost_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -791,6 +791,70 @@ func TestRuntimeCost(t *testing.T) {
791791
in: map[string]any{},
792792
want: 77,
793793
},
794+
{
795+
name: "list map literal",
796+
expr: `[{'k1': 1}, {'k2': 2}].all(x, true)`,
797+
vars: []*decls.VariableDecl{},
798+
in: map[string]any{},
799+
want: 77,
800+
},
801+
{
802+
name: ".filter list literal",
803+
expr: `[1,2,3,4,5].filter(x, x % 2 == 0)`,
804+
vars: []*decls.VariableDecl{},
805+
in: map[string]any{},
806+
want: 62,
807+
},
808+
{
809+
name: ".map list literal",
810+
expr: `[1,2,3,4,5].map(x, x)`,
811+
vars: []*decls.VariableDecl{},
812+
in: map[string]any{},
813+
want: 86,
814+
},
815+
{
816+
name: ".map.filter list literal",
817+
expr: `[1,2,3,4,5].map(x, x).filter(x, x % 2 == 0)`,
818+
vars: []*decls.VariableDecl{},
819+
in: map[string]any{},
820+
want: 138,
821+
},
822+
{
823+
name: ".map.exists list literal",
824+
expr: `[1,2,3,4,5].map(x, x).exists(x, x == 5) == true`,
825+
vars: []*decls.VariableDecl{},
826+
in: map[string]any{},
827+
want: 118,
828+
},
829+
{
830+
name: ".map.map list literal",
831+
expr: `[1,2,3,4,5].map(x, x).map(x, x)`,
832+
vars: []*decls.VariableDecl{},
833+
in: map[string]any{},
834+
want: 162,
835+
},
836+
{
837+
name: ".map.map list literal",
838+
expr: `[1,2,3,4,5].map(x, [x, x]).filter(z, z.size() == 2)`,
839+
vars: []*decls.VariableDecl{},
840+
in: map[string]any{},
841+
want: 232,
842+
},
843+
{
844+
name: "comprehension on nested list",
845+
expr: `[1,2,3,4,5].map(x, [x, x]).all(y, y.all(y, y == 1))`,
846+
want: 171,
847+
},
848+
{
849+
name: "comprehension size",
850+
expr: `[1,2,3,4,5].map(x, x).map(x, x) + [1]`,
851+
want: 173,
852+
},
853+
{
854+
name: "nested comprehension",
855+
expr: `[1,2,3].all(i, i in [1,2,3].map(j, j + j))`,
856+
want: 86,
857+
},
794858
}
795859

796860
for _, tc := range cases {

0 commit comments

Comments
 (0)