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/01/18 09:16:04 UTC

[GitHub] [arrow] vibhatha opened a new pull request #12175: ARROW-15154: [R] Expose ReferencedBufferSize to R [WIP]

vibhatha opened a new pull request #12175:
URL: https://github.com/apache/arrow/pull/12175


   In this PR, the `ReferencedBufferSize` functionality exposed in R API 
   
   This is integrated for 
   
   - [x] Array
   - [ ] ChunkedArray
   - [ ] RecordBatch
   - [ ] Table


-- 
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] vibhatha commented on pull request #12175: ARROW-15154: [R] Expose ReferencedBufferSize to R

Posted by GitBox <gi...@apache.org>.
vibhatha commented on pull request #12175:
URL: https://github.com/apache/arrow/pull/12175#issuecomment-1017088583


   @pitrou thank you for the review comments. I will address them :) 


-- 
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 edited a comment on pull request #12175: ARROW-15154: [R] Expose ReferencedBufferSize to R

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12175:
URL: https://github.com/apache/arrow/pull/12175#issuecomment-1018683282


   Benchmark runs are scheduled for baseline = 3fa2d2171a11e1823036a842ec99fd010014eb82 and contender = e291d0f408df588048b6971819fc4e65b0093154. e291d0f408df588048b6971819fc4e65b0093154 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/cfd5dfc92fa14efeb19d2a78fa6e450b...ed79a72fe18043b184244ded3c989007/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/56e9c4d2228a4e5f905629d9d5d30858...0a2afeef823a42a3b9160fc5d31ade18/)
   [Finished :arrow_down:0.17% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/5561ee137fdc4afaa64357dc42f9ef83...deec4444a6354ef08cbc1f273215a16a/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
   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] ursabot edited a comment on pull request #12175: ARROW-15154: [R] Expose ReferencedBufferSize to R

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12175:
URL: https://github.com/apache/arrow/pull/12175#issuecomment-1018683282


   Benchmark runs are scheduled for baseline = 3fa2d2171a11e1823036a842ec99fd010014eb82 and contender = e291d0f408df588048b6971819fc4e65b0093154. e291d0f408df588048b6971819fc4e65b0093154 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/cfd5dfc92fa14efeb19d2a78fa6e450b...ed79a72fe18043b184244ded3c989007/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/56e9c4d2228a4e5f905629d9d5d30858...0a2afeef823a42a3b9160fc5d31ade18/)
   [Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/5561ee137fdc4afaa64357dc42f9ef83...deec4444a6354ef08cbc1f273215a16a/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
   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] vibhatha commented on pull request #12175: ARROW-15154: [R] Expose ReferencedBufferSize to R

Posted by GitBox <gi...@apache.org>.
vibhatha commented on pull request #12175:
URL: https://github.com/apache/arrow/pull/12175#issuecomment-1018922254


   Thanks a lot @thisisnic ☺️


-- 
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] thisisnic closed pull request #12175: ARROW-15154: [R] Expose ReferencedBufferSize to R

Posted by GitBox <gi...@apache.org>.
thisisnic closed pull request #12175:
URL: https://github.com/apache/arrow/pull/12175


   


-- 
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] vibhatha commented on pull request #12175: ARROW-15154: [R] Expose ReferencedBufferSize to R

Posted by GitBox <gi...@apache.org>.
vibhatha commented on pull request #12175:
URL: https://github.com/apache/arrow/pull/12175#issuecomment-1018591872


   @thisisnic I updated the PR 


-- 
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 #12175: ARROW-15154: [R] Expose ReferencedBufferSize to R [WIP]

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12175:
URL: https://github.com/apache/arrow/pull/12175#issuecomment-1015213943






-- 
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 edited a comment on pull request #12175: ARROW-15154: [R] Expose ReferencedBufferSize to R

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12175:
URL: https://github.com/apache/arrow/pull/12175#issuecomment-1018683282


   Benchmark runs are scheduled for baseline = 3fa2d2171a11e1823036a842ec99fd010014eb82 and contender = e291d0f408df588048b6971819fc4e65b0093154. e291d0f408df588048b6971819fc4e65b0093154 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/cfd5dfc92fa14efeb19d2a78fa6e450b...ed79a72fe18043b184244ded3c989007/)
   [Failed :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/56e9c4d2228a4e5f905629d9d5d30858...0a2afeef823a42a3b9160fc5d31ade18/)
   [Finished :arrow_down:0.17% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/5561ee137fdc4afaa64357dc42f9ef83...deec4444a6354ef08cbc1f273215a16a/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
   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] vibhatha commented on pull request #12175: ARROW-15154: [R] Expose ReferencedBufferSize to R

Posted by GitBox <gi...@apache.org>.
vibhatha commented on pull request #12175:
URL: https://github.com/apache/arrow/pull/12175#issuecomment-1017549553


   @pitrou updated the PR 


-- 
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 change in pull request #12175: ARROW-15154: [R] Expose ReferencedBufferSize to R

Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #12175:
URL: https://github.com/apache/arrow/pull/12175#discussion_r787699385



##########
File path: r/src/chunkedarray.cpp
##########
@@ -143,4 +144,10 @@ std::shared_ptr<arrow::ChunkedArray> ChunkedArray__from_list(cpp11::list chunks,
   return std::make_shared<arrow::ChunkedArray>(std::move(vec));
 }
 
+// [[arrow::export]]
+int64_t ChunkedArray__ReferencedBufferSize(const std::shared_ptr<arrow::ChunkedArray>& x) {
+  auto chunked_array = arrow::internal::checked_cast<const arrow::ChunkedArray*>(x.get());

Review comment:
       Same here.

##########
File path: r/src/recordbatch.cpp
##########
@@ -305,4 +307,10 @@ std::shared_ptr<arrow::RecordBatch> RecordBatch__from_arrays(SEXP schema_sxp, SE
   return arrow::RecordBatch::Make(schema, num_rows, arrays);
 }
 
+// [[arrow::export]]
+int64_t RecordBatch__ReferencedBufferSize(const std::shared_ptr<arrow::RecordBatch>& x) {
+  auto batch = arrow::internal::checked_cast<const arrow::RecordBatch*>(x.get());

Review comment:
       Here too.

##########
File path: r/src/array.cpp
##########
@@ -288,4 +289,10 @@ bool Array__Same(const std::shared_ptr<arrow::Array>& x,
                  const std::shared_ptr<arrow::Array>& y) {
   return x.get() == y.get();
 }
+
+// [[arrow::export]]
+int64_t Array__ReferencedBufferSize(const std::shared_ptr<arrow::Array>& x) {
+  auto array = arrow::internal::checked_cast<const arrow::Array*>(x.get());
+  return ValueOrStop(arrow::util::ReferencedBufferSize(*array));

Review comment:
       The checked_cast is pointless here, the type is the same left and right.
   ```suggestion
     return ValueOrStop(arrow::util::ReferencedBufferSize(*x));
   ```

##########
File path: r/src/table.cpp
##########
@@ -21,6 +21,7 @@
 #include <arrow/array/array_base.h>
 #include <arrow/table.h>
 #include <arrow/util/key_value_metadata.h>
+#include <arrow/util/byte_size.h>

Review comment:
       Alphebetical ordering here too.

##########
File path: r/src/recordbatch.cpp
##########
@@ -25,6 +25,8 @@
 #include <arrow/ipc/writer.h>
 #include <arrow/type.h>
 #include <arrow/util/key_value_metadata.h>
+#include <arrow/util/byte_size.h>

Review comment:
       Can you keep these includes alphabetically-ordered?

##########
File path: r/src/table.cpp
##########
@@ -283,4 +284,10 @@ std::shared_ptr<arrow::Table> Table__from_record_batches(
   return tab;
 }
 
+// [[arrow::export]]
+int64_t Table__ReferencedBufferSize(const std::shared_ptr<arrow::Table>& x) {
+  auto table = arrow::internal::checked_cast<const arrow::Table*>(x.get());

Review comment:
       Casting here too.




-- 
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] vibhatha commented on pull request #12175: ARROW-15154: [R] Expose ReferencedBufferSize to R

Posted by GitBox <gi...@apache.org>.
vibhatha commented on pull request #12175:
URL: https://github.com/apache/arrow/pull/12175#issuecomment-1018508787


   @thisisnic thank you very much for the review. I will update the PR. 


-- 
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 #12175: ARROW-15154: [R] Expose ReferencedBufferSize to R

Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #12175:
URL: https://github.com/apache/arrow/pull/12175#issuecomment-1018683282


   Benchmark runs are scheduled for baseline = 3fa2d2171a11e1823036a842ec99fd010014eb82 and contender = e291d0f408df588048b6971819fc4e65b0093154. e291d0f408df588048b6971819fc4e65b0093154 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/cfd5dfc92fa14efeb19d2a78fa6e450b...ed79a72fe18043b184244ded3c989007/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/56e9c4d2228a4e5f905629d9d5d30858...0a2afeef823a42a3b9160fc5d31ade18/)
   [Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/5561ee137fdc4afaa64357dc42f9ef83...deec4444a6354ef08cbc1f273215a16a/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
   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] vibhatha commented on a change in pull request #12175: ARROW-15154: [R] Expose ReferencedBufferSize to R

Posted by GitBox <gi...@apache.org>.
vibhatha commented on a change in pull request #12175:
URL: https://github.com/apache/arrow/pull/12175#discussion_r788324172



##########
File path: r/src/recordbatch.cpp
##########
@@ -25,6 +25,8 @@
 #include <arrow/ipc/writer.h>
 #include <arrow/type.h>
 #include <arrow/util/key_value_metadata.h>
+#include <arrow/util/byte_size.h>

Review comment:
       Sure, will update this. 




-- 
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