Skip to content

Cleanup SlicePositionsAppender#12822

Merged
sopel39 merged 3 commits intotrinodb:masterfrom
starburstdata:ls/021-slice-pa-currentOffset
Jun 30, 2022
Merged

Cleanup SlicePositionsAppender#12822
sopel39 merged 3 commits intotrinodb:masterfrom
starburstdata:ls/021-slice-pa-currentOffset

Conversation

@lukasz-stec
Copy link
Copy Markdown
Member

Description

Is this change a fix, improvement, new feature, refactoring, or other?

improvement

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

core query engine

How would you describe this change to a non-technical end user or system administrator?

Code refactoring

Related issues, pull requests, and links

In response to discussion here

Documentation

( ) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

( ) No release notes entries required.
( ) Release notes entries required with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Jun 13, 2022
@lukasz-stec lukasz-stec force-pushed the ls/021-slice-pa-currentOffset branch from af676b9 to d647186 Compare June 14, 2022 08:43
@lukasz-stec lukasz-stec changed the title Remove unnecessary SlicePositionsAppender.currentOffset Cleanup SlicePositionsAppender Jun 15, 2022
@lukasz-stec lukasz-stec force-pushed the ls/021-slice-pa-currentOffset branch from d647186 to a91bdaa Compare June 15, 2022 07:51
@lukasz-stec lukasz-stec marked this pull request as ready for review June 27, 2022 10:40
@lukasz-stec lukasz-stec requested a review from sopel39 June 27, 2022 10:40
@lukasz-stec
Copy link
Copy Markdown
Member Author

I ran a subset of BenchmarkPartitionedOutputOperator jmh to confirm no regressions cc @sopel39

before
Benchmark                                   (channelCount)  (enableCompression)  (nullRate)  (partitionCount)  (positionCount)   (type)  Mode  Cnt     Score    Error  Units
BenchmarkPartitionedOutputOperator.addPage               1                false           0                16             8192  VARCHAR  avgt   10  2218.089 ± 32.786  ms/op
BenchmarkPartitionedOutputOperator.addPage               1                false         0.2                16             8192  VARCHAR  avgt   10  2028.034 ± 24.532  ms/op

after
Benchmark                                   (channelCount)  (enableCompression)  (nullRate)  (partitionCount)  (positionCount)   (type)  Mode  Cnt     Score    Error  Units
BenchmarkPartitionedOutputOperator.addPage               1                false           0                16             8192  VARCHAR  avgt   10  2217.335 ± 30.768  ms/op
BenchmarkPartitionedOutputOperator.addPage               1                false         0.2                16             8192  VARCHAR  avgt   10  2035.245 ± 29.503  ms/op

Remove unnecessary SlicePositionsAppender.currentOffset
+ small cleanups.
@lukasz-stec lukasz-stec force-pushed the ls/021-slice-pa-currentOffset branch from a91bdaa to 01815b2 Compare June 29, 2022 16:32
@lukasz-stec
Copy link
Copy Markdown
Member Author

@sopel39 I divided the changes in separate commits

@sopel39 sopel39 merged commit 2698c6a into trinodb:master Jun 30, 2022
@github-actions github-actions bot added this to the 389 milestone Jun 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants