You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/05/27 06:11:31 UTC
[GitHub] [arrow] zhztheplayer opened a new pull request, #13251: ARROW-16676: [C++] Wrong result of ReservationListenableMemoryPool::Impl::bytes_allocated()
zhztheplayer opened a new pull request, #13251:
URL: https://github.com/apache/arrow/pull/13251
ReservationListenableMemoryPool is a decorator of the underlying memory pool with its own mem counter. It should represent for the bytes reserved from itself rather than the underlying pool.
E.g. a ReservationListenableMemoryPool `pool-a` instance wraps the default pool, and default pool's allocation is now 1GB, `pool-a`'s allocation is 10MB, we should let `pool-a.bytes_allocated()` return 10MB rather than 1GB.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou closed pull request #13251: ARROW-16676: [C++] ReservationListenableMemoryPool::Impl::bytes_allocated() should return its own number of bytes rather than the underlying pool's
Posted by GitBox <gi...@apache.org>.
pitrou closed pull request #13251: ARROW-16676: [C++] ReservationListenableMemoryPool::Impl::bytes_allocated() should return its own number of bytes rather than the underlying pool's
URL: https://github.com/apache/arrow/pull/13251
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #13251: ARROW-16676: [C++] Wrong result of ReservationListenableMemoryPool::Impl::bytes_allocated()
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13251:
URL: https://github.com/apache/arrow/pull/13251#issuecomment-1139315717
:warning: Ticket **has not been started in JIRA**, please click 'Start Progress'.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #13251: ARROW-16676: [C++] ReservationListenableMemoryPool::Impl::bytes_allocated() should return its own number of bytes rather than the underlying pool's
Posted by GitBox <gi...@apache.org>.
pitrou commented on PR #13251:
URL: https://github.com/apache/arrow/pull/13251#issuecomment-1141871493
There's a compilation failure on the JNI CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] zhztheplayer commented on a diff in pull request #13251: ARROW-16676: [C++] ReservationListenableMemoryPool::Impl::bytes_allocated() should return its own number of bytes rather than the underlying pool's
Posted by GitBox <gi...@apache.org>.
zhztheplayer commented on code in PR #13251:
URL: https://github.com/apache/arrow/pull/13251#discussion_r885155336
##########
cpp/src/jni/dataset/jni_util.cc:
##########
@@ -108,12 +108,15 @@ class ReservationListenableMemoryPool::Impl {
}
int64_t bytes_granted = (new_block_count - blocks_reserved_) * block_size_;
blocks_reserved_ = new_block_count;
+ if (bytes_reserved_ > max_bytes_reserved_) {
+ max_bytes_reserved_ = bytes_reserved_;
+ }
return bytes_granted;
}
- int64_t bytes_allocated() { return pool_->bytes_allocated(); }
+ int64_t bytes_allocated() { return bytes_reserved_; }
Review Comment:
fixed by using `MemoryPoolStats`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #13251: ARROW-16676: [C++] ReservationListenableMemoryPool::Impl::bytes_allocated() should return its own number of bytes rather than the underlying pool's
Posted by GitBox <gi...@apache.org>.
pitrou commented on PR #13251:
URL: https://github.com/apache/arrow/pull/13251#issuecomment-1142071439
Thank you @zhztheplayer !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #13251: ARROW-16676: [C++] ReservationListenableMemoryPool::Impl::bytes_allocated() should return its own number of bytes rather than the underlying pool's
Posted by GitBox <gi...@apache.org>.
pitrou commented on code in PR #13251:
URL: https://github.com/apache/arrow/pull/13251#discussion_r884962263
##########
cpp/src/jni/dataset/jni_util.cc:
##########
@@ -108,12 +108,15 @@ class ReservationListenableMemoryPool::Impl {
}
int64_t bytes_granted = (new_block_count - blocks_reserved_) * block_size_;
blocks_reserved_ = new_block_count;
+ if (bytes_reserved_ > max_bytes_reserved_) {
+ max_bytes_reserved_ = bytes_reserved_;
+ }
return bytes_granted;
}
- int64_t bytes_allocated() { return pool_->bytes_allocated(); }
+ int64_t bytes_allocated() { return bytes_reserved_; }
Review Comment:
(note this is not thread-safe)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #13251: ARROW-16676: [C++] Wrong result of ReservationListenableMemoryPool::Impl::bytes_allocated()
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13251:
URL: https://github.com/apache/arrow/pull/13251#issuecomment-1139315711
https://issues.apache.org/jira/browse/ARROW-16676
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #13251: ARROW-16676: [C++] ReservationListenableMemoryPool::Impl::bytes_allocated() should return its own number of bytes rather than the underlying pool's
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #13251:
URL: https://github.com/apache/arrow/pull/13251#issuecomment-1142636699
Benchmark runs are scheduled for baseline = ddf5436fa883a77188d9a34795008551b45431cf and contender = 931422bb9cba959d18d7bd4c47536317f1d7f3e6. 931422bb9cba959d18d7bd4c47536317f1d7f3e6 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/f0aebab748e042d9a9655b8190e6af06...3b2e3fc152a1456f99e35dff5905992c/)
[Failed :arrow_down:0.5% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/69a9ea17d9e44a5abbd028874f55c152...c1f39f2073ac4bc7877667af394f718d/)
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/128d3f73b7554d4fb509a3003b1c0c0f...9d7b6c40255d488ab93626d54784428a/)
[Finished :arrow_down:0.39% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/19db36298bd14a1c9b0af853f0352b77...738d082ab8d2458ca1430cbb9d0acd33/)
Buildkite builds:
[Finished] [`931422bb` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/854)
[Failed] [`931422bb` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/854)
[Finished] [`931422bb` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/844)
[Finished] [`931422bb` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/856)
[Finished] [`ddf5436f` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/853)
[Finished] [`ddf5436f` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/853)
[Finished] [`ddf5436f` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/843)
[Finished] [`ddf5436f` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/855)
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #13251: ARROW-16676: [C++] ReservationListenableMemoryPool::Impl::bytes_allocated() should return its own number of bytes rather than the underlying pool's
Posted by GitBox <gi...@apache.org>.
pitrou commented on PR #13251:
URL: https://github.com/apache/arrow/pull/13251#issuecomment-1142026174
Rebased and fixed code formatting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org