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 2020/07/31 16:14:27 UTC

[GitHub] [arrow] vertexclique opened a new pull request #7874: ARROW-9582: [Rust] Implement memory size methods

vertexclique opened a new pull request #7874:
URL: https://github.com/apache/arrow/pull/7874


   This PR is a slightly extended version of the PR https://github.com/apache/arrow/pull/7853.
   
   * `buffer_memory_size`: Only calculates internally held data size.
   * `total_memory_size`: Calculates total physical memory size. (I am not sure about the name)
   
   cc @andygrove @paddyhoran @nevi-me 


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] andygrove commented on pull request #7874: ARROW-9582: [Rust] Implement memory size methods

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


   I agree with @jhorstmann since I am also confused by the naming and comments.
   
   My original requirement was for a method that would tell me how much physical memory is occupied by an array's buffers. I would call this something like `get_buffer_memory_size`. 
   
   Others than asked for additional functionality to determine how much of the memory is actually being used for data. I suppose I would call this something like `get_buffer_utitilized_size`.
   
   Then there is the idea of an additional method that returns the total memory used by an array, including the buffers and other attributes. I'm not sure why we need this, personally, but if we do then I suppose `get_array_memory_size` would work?
   
   What do others think?
   
   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] vertexclique commented on pull request #7874: ARROW-9582: [Rust] Implement memory size methods

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


   Is this ok @andygrove ? Can we merge this one as it is?


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] vertexclique edited a comment on pull request #7874: ARROW-9582: [Rust] Implement memory size methods

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


   > My original requirement was for a method that would tell me how much physical memory is occupied by an array's buffers. I would call this something like `get_buffer_memory_size`.
   
   > Others than asked for additional functionality to determine how much of the memory is actually being used for data. I suppose I would call this something like `get_buffer_utitilized_size`.
   
   > Then there is the idea of an additional method that returns the total memory used by an array, including the buffers and other attributes. I'm not sure why we need this, personally, but if we do then I suppose `get_array_memory_size` would work?
   
   1. Changed to `get_buffer_memory_size` and `get_array_memory_size`. Since they correspond to that.
   
   > ... I'm not sure why we need this, personally...
   
   I am using this for memory estimation and eviction calculation in the db that I am writing. That is kind of important since these are also giving accurate results about what is resident.
   
   plus this is how servo calculates actual memory usage.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] vertexclique commented on pull request #7874: ARROW-9582: [Rust] Implement memory size methods

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


   > My original requirement was for a method that would tell me how much physical memory is occupied by an array's buffers. I would call this something like `get_buffer_memory_size`.
   
   > Others than asked for additional functionality to determine how much of the memory is actually being used for data. I suppose I would call this something like `get_buffer_utitilized_size`.
   
   > Then there is the idea of an additional method that returns the total memory used by an array, including the buffers and other attributes. I'm not sure why we need this, personally, but if we do then I suppose `get_array_memory_size` would work?
   
   1. Changed to `get_buffer_memory_size` and `get_array_memory_size`. Since they correspond to that.
   
   > ... I'm not sure why we need this, personally...
   
   I am using this for memory estimation and eviction calculation in the db that I am writing. That is kind of important since these are also giving accurate results about what is resident.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] github-actions[bot] commented on pull request #7874: ARROW-9582: [Rust] Implement memory size methods

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


   https://issues.apache.org/jira/browse/ARROW-9582


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] sunchao commented on pull request #7874: ARROW-9582: [Rust] Implement memory size methods

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


   Instead of `buffer_memory_size` and `total_memory_size`, I'm thinking whether `memory_used` and `memory_capacity` makes more sense. 


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] vertexclique commented on pull request #7874: ARROW-9582: [Rust] Implement memory size methods

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


   @sunchao even the latter is physical memory calculation, I addressed your comment. :)


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jhorstmann commented on pull request #7874: ARROW-9582: [Rust] Implement memory size methods

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


   > Instead of `buffer_memory_size` and `total_memory_size`, I'm thinking whether `memory_used` and `memory_capacity` makes more sense.
   
   I think the new names, or maybe the comments for those methods, are more confusing than before. The idea seemed to be to have one method returning the size of just the buffers and another method that includes additional memory overhead like pointers, offsets, arc and so on.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jhorstmann commented on pull request #7874: ARROW-9582: [Rust] Implement memory size methods

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


   The names proposed by @andygrove sound very clear to me. I also don't think there is a need for a method that includes additional attributes as that overhead should be comparatively low in applications that care about memory usage, and it is maybe not worth the additional code complexity.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] andygrove closed pull request #7874: ARROW-9582: [Rust] Implement memory size methods

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


   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] vertexclique edited a comment on pull request #7874: ARROW-9582: [Rust] Implement memory size methods

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


   @sunchao even the latter is physical memory calculation, I addressed your comment (that's also shorter naming). :)


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org