[FLINK-12335][Table-runtime]Remove useless code in class SegmentsUtil

[liyafan82]

…entsUtil

What is the purpose of the change

Improve the performance of SegmentsUtil class :

To evaluate the offset, an integer is bitand with a mask to clear to low bits, and then shift right. The bitand is useless:
((index & BIT_BYTE_POSITION_MASK) >>> 3)

Brief change log

• Change class SegmentsUtil to fix the above 2 issues.

Verifying this change

Verified by manually running existing test cases.

Does this pull request potentially affect one of the following parts:

• Dependencies (does it add or upgrade a dependency): (no)
• The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
• The serializers: (no)
• The runtime per-record code paths (performance sensitive): (yes)
• Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
• The S3 file system connector: (no)

Documentation

• Does this pull request introduce a new feature? (no)

[flinkbot]

Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.

Review Progress

• ❓ 1. The [description] looks good.
• ❓ 2. There is [consensus] that the contribution should go into to Flink.
• ❗ 3. Needs [attention] from.
  • Needs attention by @wuchong [committer]
• ❓ 4. The change fits into the overall [architecture].
• ❓ 5. Overall code [quality] is good.

Please see the Pull Request Review Guide for a full explanation of the review process.

The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required

Bot commands

The @flinkbot bot supports the following commands:

• @flinkbot approve description to approve one or more aspects (aspects: description, consensus, architecture and quality)
• @flinkbot approve all to approve all aspects
• @flinkbot approve-until architecture to approve everything until architecture
• @flinkbot attention @username1 [@username2 ..] to require somebody's attention
• @flinkbot disapprove architecture to remove an approval you gave earlier

[JingsongLi]

Thanks @liyafan82 for you code, maybe you can change the title to remove useless code in SegmentsUtil, I think the jdk/jvm will optimizer it, so maybe performance keep same.

[JingsongLi]

Can you and a util method like this:

/**
     * Given a bit index, return word index containing it.
     */
    private static int wordIndex(int bitIndex) {
        return bitIndex >> ADDRESS_BITS_PER_WORD;
    }

[liyafan82]

Good suggestion. Thank you.

[JingsongLi]

It seems that BIT_BYTE_INDEX_MASK is not needed either.

[liyafan82]

Good point. Thanks for your comments.

[JingsongLi]

We can't remove BIT_BYTE_INDEX_MASK, the 1 << is int operation, but current &= should be byte operation.

[liyafan82]

Thanks @liyafan82 for you code, maybe you can change the title to remove useless code in SegmentsUtil, I think the jdk/jvm will optimizer it, so maybe performance keep same.

Thanks for your comments.

I have made some investigations to analyze the assembly generated by JIT.
It seems that JIT successfully removes BIT_BYTE_POSITION_MASK, so there can be no performance overhead. This PR only improves the code structure.

[JingsongLi]

Thanks @liyafan82 , LGTM +1

[JingsongLi]

@flinkbot attention @wuchong

[wuchong]

LGTM.

[wuchong]

LGTM.

[KurtYoung]

Cool, thanks @liyafan82 for your contributions.