RibDelta optimizations

[anothermattbrown]

1. flatten actions
   Previously RibDelta._actions was a sorted map from Prefix to list of
   advertisements for that prefix. The only use of the map was to
   efficiently retrieve the keyset for getPrefixes. However, getPrefixes
   is called at most once, and often 0 times. So we should only pay to
   collect and dedup the prefixes if it's called.

2. avoid using Builders when possible
   Particularly in hot code. The builder allocates a linked hash map,
   copies all the advertisements in, and then copies them out to a list
   on build.

[batfish-bot]

This change is 

[codecov]

Codecov Report

Merging #6245 into master will decrease coverage by 0.01%.
The diff coverage is 92.85%.

@@             Coverage Diff              @@
##             master    #6245      +/-   ##
============================================
- Coverage     72.98%   72.97%   -0.02%     
- Complexity    35119    35120       +1     
============================================
  Files          2837     2837              
  Lines        142729   142742      +13     
  Branches      17122    17129       +7     
============================================
- Hits         104176   104159      -17     
- Misses        30318    30337      +19     
- Partials       8235     8246      +11     

Impacted Files	Coverage Δ	Complexity Δ
.../org/batfish/dataplane/rib/RouteAdvertisement.java	83.67% <66.66%> (-5.46%)	18.00 <2.00> (+1.00)	⬇️
...c/main/java/org/batfish/dataplane/rib/RibTree.java	88.33% <86.95%> (-5.42%)	28.00 <5.00> (ø)
.../main/java/org/batfish/dataplane/rib/RibDelta.java	91.66% <96.29%> (-5.04%)	32.00 <12.00> (+3.00)	⬇️
...n/java/org/batfish/common/util/CollectionUtil.java	92.30% <100.00%> (+3.84%)	18.00 <5.00> (+5.00)
...java/org/batfish/dataplane/ibdp/VirtualRouter.java	87.70% <100.00%> (-0.21%)	265.00 <0.00> (ø)
...rg/batfish/identifiers/StorageBasedIdResolver.java	88.88% <0.00%> (-4.45%)	27.00% <0.00%> (ø%)
...batfish/representation/aws/LoadBalancerTarget.java	54.16% <0.00%> (-4.17%)	7.00% <0.00%> (-2.00%)
... and 9 more

[dhalperi]

Reviewed 6 of 7 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @anothermattbrown and @progwriter)

---

projects/batfish/src/main/java/org/batfish/dataplane/rib/RibDelta.java, line 51 at r2 (raw file):

   */
  public static <R extends AbstractRouteDecorator> RibDelta<R> adding(Collection<R> routes) {
    return new RibDelta<>(

Should we initialize a builder with size instead? Will save in intermediate allocations. builderWithExpectedSize

---

projects/batfish/src/main/java/org/batfish/dataplane/rib/RibDelta.java, line 126 at r2 (raw file):

@SuppressWarnings("PMD.LooseCoupling") // insertion order matters

FYI this one

---

projects/batfish/src/main/java/org/batfish/dataplane/rib/RibDelta.java, line 136 at r2 (raw file):

 private Map<R, RouteAdvertisement<R>> _actions;

FWIW, in prior iterations of this code I preferred the supresswarnings of PMD.LooseCoupling and keeping explicit type signature. I commented at it.

[dhalperi]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @anothermattbrown and @progwriter)

---

projects/batfish/src/main/java/org/batfish/dataplane/rib/RibDelta.java, line 51 at r2 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

Should we initialize a builder with size instead? Will save in intermediate allocations. builderWithExpectedSize

counterpoint: we could confirm whether the collector takes advantage of this.

[dhalperi]

---

projects/batfish/src/main/java/org/batfish/dataplane/rib/RibDelta.java, line 51 at r2 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

counterpoint: we could confirm whether the collector takes advantage of this.

Anti-counter-point: it does not.

[dhalperi]

Reviewed 1 of 1 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @progwriter)

[progwriter]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @progwriter)