Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARROW-17940: [Java][Gandiva] Implement Reserve for JavaBuffer #14323

Merged

Conversation

js8544
Copy link
Collaborator

@js8544 js8544 commented Oct 5, 2022

See previous discussions at #14261

@github-actions
Copy link

github-actions bot commented Oct 5, 2022

@github-actions
Copy link

github-actions bot commented Oct 5, 2022

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@js8544
Copy link
Collaborator Author

js8544 commented Oct 5, 2022

This PR is based on changes from #14310 so will need to wait for that to merge.

@js8544
Copy link
Collaborator Author

js8544 commented Oct 5, 2022

@github-actions crossbow submit java-jars

@github-actions
Copy link

github-actions bot commented Oct 5, 2022

Revision: 3b1ffe4

Submitted crossbow builds: ursacomputing/crossbow @ actions-d680acd16f

Task Status
java-jars Github Actions

@@ -15,6 +15,7 @@
// specific language governing permissions and limitations
// under the License.

#include <arrow/status.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move this include with the other Arrow includes below?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

return Status::NotImplemented("shrink not implemented");
}

if (ARROW_PREDICT_TRUE(new_size < capacity())) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (ARROW_PREDICT_TRUE(new_size < capacity())) {
if (ARROW_PREDICT_TRUE(new_size <= capacity())) {

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

capacity_ = ret_capacity;
return Status::OK();
}

Status JavaResizableBuffer::Resize(const int64_t new_size, bool shrink_to_fit) {
if (shrink_to_fit == true) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we don't need to implement this because it's not used currently?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's not needed by gandiva

@js8544 js8544 force-pushed the jinshang/implement_reserve_for_gandiva_java_buffer branch from 3b1ffe4 to d90fcf0 Compare October 9, 2022 02:42
@js8544 js8544 marked this pull request as ready for review October 9, 2022 02:43
@js8544
Copy link
Collaborator Author

js8544 commented Oct 9, 2022

@github-actions crossbow submit java-jars

@github-actions
Copy link

github-actions bot commented Oct 9, 2022

Revision: d90fcf0

Submitted crossbow builds: ursacomputing/crossbow @ actions-73726f002e

Task Status
java-jars Github Actions

@js8544 js8544 force-pushed the jinshang/implement_reserve_for_gandiva_java_buffer branch from d90fcf0 to 34c7061 Compare October 11, 2022 02:10
@js8544
Copy link
Collaborator Author

js8544 commented Oct 11, 2022

@github-actions crossbow submit java-jars

@github-actions
Copy link

Revision: 34c7061

Submitted crossbow builds: ursacomputing/crossbow @ actions-2a8cd8ba9f

Task Status
java-jars Github Actions

@js8544 js8544 requested a review from pitrou October 11, 2022 04:17
@pitrou pitrou merged commit 49a53d2 into apache:master Oct 11, 2022
@js8544 js8544 deleted the jinshang/implement_reserve_for_gandiva_java_buffer branch October 11, 2022 08:43
@ursabot
Copy link

ursabot commented Oct 11, 2022

Benchmark runs are scheduled for baseline = e21ae28 and contender = 49a53d2. 49a53d2 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
[Failed ⬇️0.56% ⬆️0.0%] test-mac-arm
[Failed ⬇️1.1% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.54% ⬆️0.0%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 49a53d2f ec2-t3-xlarge-us-east-2
[Failed] 49a53d2f test-mac-arm
[Failed] 49a53d2f ursa-i9-9960x
[Finished] 49a53d2f ursa-thinkcentre-m75q
[Finished] e21ae28c ec2-t3-xlarge-us-east-2
[Failed] e21ae28c test-mac-arm
[Failed] e21ae28c ursa-i9-9960x
[Finished] e21ae28c 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
Copy link

ursabot commented Oct 11, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants