Replace emptiness checks with isEmpty()

[richardstartin]

Description

The vast majority if not all of these will be benign, but this change set prevents emptiness checks from becoming bottlenecks if collection implementation is changed. Applies similar changes to manual emptiness checks performed on Strings.

Upgrade Notes

Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)

• Yes (Please label as backward-incompat, and complete the section below on Release Notes)

Does this PR fix a zero-downtime upgrade introduced earlier?

• Yes (Please label this as backward-incompat, and complete the section below on Release Notes)

Does this PR otherwise need attention when creating release notes? Things to consider:

• New configuration options
• Deprecation of configurations
• Signature changes to public methods/interfaces
• New plugins added or old plugins removed

• Yes (Please label this PR as release-notes and complete the section on Release Notes)

Release Notes

Documentation

[richardstartin]

Just in case the String change is contentious, this will definitely not cause a performance regression:

@State(Scope.Benchmark)
public class StringEmptyBenchmark {

  @Param({"", "not empty"})
  String string;

  @Benchmark
  public boolean isEmpty() {
    return string.isEmpty();
  }

  @Benchmark
  public boolean equalsEmpty() {
    return "".equals(string);
  }
}

Benchmark                          (string)  Mode  Cnt  Score   Error  Units
StringEmptyBenchmark.equalsEmpty             avgt    5  3.720 ± 0.657  ns/op
StringEmptyBenchmark.equalsEmpty  not empty  avgt    5  2.671 ± 0.011  ns/op
StringEmptyBenchmark.isEmpty                 avgt    5  2.017 ± 0.016  ns/op
StringEmptyBenchmark.isEmpty      not empty  avgt    5  1.997 ± 0.009  ns/op

[richardstartin]

Flaky test?

Error:  Failures: 
8701
Error:    NotEqualsTransformFunctionTest>BinaryOperatorTransformFunctionTest.testBinaryOperatorTransformFunction:77->BaseTransformFunctionTest.testTransformFunction:206 expected [0] but found [1]

This passes locally. Rerun CI?

[codecov-commenter]

Codecov Report

Merging #7468 (f3b57a5) into master (72d05a7) will decrease coverage by 42.86%.
The diff coverage is 19.44%.

❗ Current head f3b57a5 differs from pull request most recent head 11f09f1. Consider uploading reports for the commit 11f09f1 to get more accurate results

@@              Coverage Diff              @@
##             master    #7468       +/-   ##
=============================================
- Coverage     71.93%   29.07%   -42.87%     
=============================================
  Files          1519     1510        -9     
  Lines         75341    74994      -347     
  Branches      10985    10947       -38     
=============================================
- Hits          54198    21804    -32394     
- Misses        17498    51207    +33709     
+ Partials       3645     1983     -1662     

Flag	Coverage Δ
integration1	?
integration2	29.07% <19.44%> (-0.07%)	⬇️
unittests1	?
unittests2	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...n/java/org/apache/pinot/client/BrokerResponse.java	83.33% <0.00%> (-16.67%)	⬇️
...ta/segment/SegmentZKMetadataCustomMapModifier.java	60.60% <0.00%> (-30.31%)	⬇️
.../apache/pinot/common/utils/NamedThreadFactory.java	50.00% <0.00%> (ø)
...t/controller/api/resources/PinotQueryResource.java	0.00% <0.00%> (ø)
...t/controller/api/resources/TableDebugResource.java	0.00% <0.00%> (ø)
.../controller/helix/core/SegmentDeletionManager.java	25.20% <0.00%> (-49.60%)	⬇️
...les/utils/QueryInvertedSortedIndexRecommender.java	0.00% <0.00%> (-81.80%)	⬇️
...he/pinot/controller/util/AutoAddInvertedIndex.java	0.00% <0.00%> (ø)
...ot/controller/util/TableIngestionStatusHelper.java	0.00% <0.00%> (ø)
...ata/manager/offline/DimensionTableDataManager.java	0.00% <0.00%> (-88.89%)	⬇️
... and 1078 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 72d05a7...11f09f1. Read the comment docs.

[Jackie-Jiang]

LGTM. Re-run the tests. I feel the test failure is caused by float ser/de not returning the same result - f != Float.parseFloat(String.format("%f", f))

[richardstartin]

I feel the test failure is caused by float ser/de not returning the same result - f != Float.parseFloat(String.format("%f", f))

Good spot, that's worth fixing before it happens again.