GH-14844: [Java] Short circuit null checks when comparing non null field types

[markjschreiber]

Relates to #14844

To avoid expensive checks for null values when the vectors cannot contain nulls, a flag is set when a vector is attached to indicate if a null check is needed when compare(idx1, idx2) is called. If it isn't then the call will immediately redirect to compareNotNull(index1, index2).

• Closes: [Java] VectorValueComparator should skip the null test for NonNullable FieldTypes types #14844

[github-actions]

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

In the case of old issues on JIRA the title also supports:

ARROW-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

See also:

• Other pull requests
• Contribution Guidelines - How to contribute patches

[github-actions]

• Closes: [Java] VectorValueComparator should skip the null test for NonNullable FieldTypes types #14844

[github-actions]

⚠️ GitHub issue #14844 has been automatically assigned in GitHub to PR creator.

[lidavidm]

It may be preferable to check for the presence/absence of a validity buffer (I think Java does the same optimization as C++ here) - that'll cover cases where a nullable vector has no nulls, too.

It won't cover cases where a non-nullable vector has a validity buffer of all 1s, but as-is I'd say we may not want to enable this by default since nothing is really verifying that the data matches that part of the schema.

[markjschreiber]

For the cases I have tested the java vectors seem to have a validity buffer even when there can be no nulls. Depending on the AllocationManager this is some form of buffer called EMPTY which is not null but cannot allocate memory for items. It's not possible the test if the buffer is equal to one of these because they are private

However, given that someone might change the implementation one day I will include a check for a null validity buffer to catch this case as well.

[lidavidm]

Ah, that's what I wasn't remembering. In that case, the right check might be to see if the buffer length is 0 (in which case there can't be any nulls)?

[markjschreiber]

Weirdly it seems that the validity buffer does have a capacity set when any values are added to the value buffer. There is a method to find the number of nulls in the vector which is relatively fast, only needs to be checked when attaching the vector and can be skipped for FieldTypes that are nonNullable

Turns out I also need to add a safety check that uses the "null safe" comparison when the vector contains no values (valueCount = 0) as the TreeBasedDictionaryBuilder seems to need this.

[lidavidm]

Ah, thanks for working through it. I suppose Java doesn't optimize the validity buffer in the same way that C++ does.

[cyb70289]

@markjschreiber ping me if you need CI approval. Most maintainers are in vacation.

[lidavidm]

Thanks!

[ursabot]

Benchmark runs are scheduled for baseline = 47c1ff1 and contender = d00f016. d00f016 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Finished ⬇️0.0% ⬆️0.0%] test-mac-arm
[Finished ⬇️1.79% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.52% ⬆️0.07%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] d00f0163 ec2-t3-xlarge-us-east-2
[Finished] d00f0163 test-mac-arm
[Finished] d00f0163 ursa-i9-9960x
[Finished] d00f0163 ursa-thinkcentre-m75q
[Finished] 47c1ff18 ec2-t3-xlarge-us-east-2
[Finished] 47c1ff18 test-mac-arm
[Finished] 47c1ff18 ursa-i9-9960x
[Finished] 47c1ff18 ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

[ursabot]

['Python', 'R'] benchmarks have high level of regressions.
ursa-i9-9960x