1713322 - Figure out a way to fill relatively low-flex buckets at low maxResultCounts

Status: RESOLVED FIXED

(Firefox :: Address Bar, task, P3)

Description

[Harry Twyford]

Follow-up bug based on this review comment.

The muxer calculates bucket fill by multiplying maxResultCount by the relative size of the flexed bucket, then rounding. In the case of the general bucket after bug 1712352, all child buckets are rounded to zero and no results are shown when maxResultCount is 1. This breaks a subtest in test_providersManager_maxResults.js.

Comment 1

[Drew Willcoxon :adw]

Attachment: Bug 1713322 - Properly fill flex groups and add support for both availableSpan and maxResultCount.

The problem in this bug is that the sum of the available spans of child buckets
is not necessarily equal to the available span of the parent bucket. That's
because each child span must be an integer, but a child's ideal span may not be
an integer, so we have to round and an error can accumulate.

This fixes it by detecting that case and then tweaking child spans until their
sum is equal to the parent's available span. If the sum is smaller than the
parent span, then we increment child spans until it's equal; if the sum is
larger than the parent span, then we decrement instead. (The case where the sum
is larger isn't as much of a problem as when it's smaller. We still correctly
limit the total result span due to logic elsewhere, but the bucket's final
result composition may not reflect flex ratios as accurately as it could.)

I added some logic so that the child spans we choose to tweak are the ones that
minimize the mathematical error between the final integer spans and the ideal
unrounded spans. In other words, we pick the best spans possible given that they
must be integers.

We need to do this tweaking when we try to overfill buckets too, not only on the
first pass for a bucket. Currently the second pass is hardcoded in
_fillBuckets. Rather than hardcoding the tweaking in both places, I added an
_updateFlexData helper method. While I was doing that, I realized we may need
more than two passes per parent bucket in order to optimally fill the children.
So I modified _fillBuckets so it recurses with the parent bucket itself when
we need to overfill any of its children.

Finally while I was here I switched from "bucket" to "group" in light of the
renaming in bug 1715484 and bug 1715822.

Comment 2

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b9ea2179c9e4e0d2a820ea22958862c87d10a92b

Comment 3

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=689ffd091a3818d0928c4d1df0bd7cdf66393b68

Comment 4

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bbe3772cca3e4ddafe13041dc4fb4235183f2ce6

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/02edf40c1679
Properly fill flex groups and add support for both availableSpan and maxResultCount. r=mak

Comment 6

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/02edf40c1679