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/10/28 12:02:55 UTC
[GitHub] [arrow] Marwes opened a new pull request #8545: ARROW-9674: [Rust] Make the parquet read and writers Send
Marwes opened a new pull request #8545:
URL: https://github.com/apache/arrow/pull/8545
Based on #8509
----------------------------------------------------------------
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] alamb closed pull request #8545: ARROW-9674: [Rust] Make the parquet read and writers Send
Posted by GitBox <gi...@apache.org>.
alamb closed pull request #8545:
URL: https://github.com/apache/arrow/pull/8545
----------------------------------------------------------------
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] alamb commented on a change in pull request #8545: ARROW-9674: [Rust] Make the parquet read and writers Send
Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #8545:
URL: https://github.com/apache/arrow/pull/8545#discussion_r527993120
##########
File path: rust/parquet/src/util/memory.rs
##########
@@ -39,37 +41,39 @@ pub type WeakMemTrackerPtr = Weak<MemTracker>;
pub struct MemTracker {
// In the tuple, the first element is the current memory allocated (in bytes),
// and the second element is the maximum memory allocated so far (in bytes).
- memory_usage: Cell<(i64, i64)>,
+ current_memory_usage: AtomicI64,
+ max_memory_usage: AtomicI64,
}
impl MemTracker {
/// Creates new memory tracker.
#[inline]
pub fn new() -> MemTracker {
MemTracker {
- memory_usage: Cell::new((0, 0)),
+ current_memory_usage: Default::default(),
+ max_memory_usage: Default::default(),
}
}
/// Returns the current memory consumption, in bytes.
pub fn memory_usage(&self) -> i64 {
- self.memory_usage.get().0
+ self.current_memory_usage.load(Ordering::Acquire)
}
/// Returns the maximum memory consumption so far, in bytes.
pub fn max_memory_usage(&self) -> i64 {
- self.memory_usage.get().1
+ self.max_memory_usage.load(Ordering::Acquire)
}
/// Adds `num_bytes` to the memory consumption tracked by this memory tracker.
#[inline]
pub fn alloc(&self, num_bytes: i64) {
- let (current, mut maximum) = self.memory_usage.get();
- let new_current = current + num_bytes;
- if new_current > maximum {
- maximum = new_current
- }
- self.memory_usage.set((new_current, maximum));
+ let new_current = self
+ .current_memory_usage
Review comment:
👍 This is a nice change to remove the use of `Cell`
----------------------------------------------------------------
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] nevi-me commented on pull request #8545: ARROW-9674: [Rust] Make the parquet read and writers Send
Posted by GitBox <gi...@apache.org>.
nevi-me commented on pull request #8545:
URL: https://github.com/apache/arrow/pull/8545#issuecomment-731429329
Beat me to the merge @alamb by seconds
----------------------------------------------------------------
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 #8545: ARROW-9674: [Rust] Make the parquet read and writers Send
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8545:
URL: https://github.com/apache/arrow/pull/8545#issuecomment-717891438
https://issues.apache.org/jira/browse/ARROW-9674
----------------------------------------------------------------
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] nevi-me commented on pull request #8545: ARROW-9674: [Rust] Make the parquet read and writers Send
Posted by GitBox <gi...@apache.org>.
nevi-me commented on pull request #8545:
URL: https://github.com/apache/arrow/pull/8545#issuecomment-731108868
The memory tracker changes are failing. Was there a big that you picked up @Marwes? Else we could isolate that, and leave the `Rc` > `Arc` changes, which I'm happy with
----------------------------------------------------------------
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] alamb commented on pull request #8545: ARROW-9674: [Rust] Make the parquet read and writers Send
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #8545:
URL: https://github.com/apache/arrow/pull/8545#issuecomment-731214207
I will try and review this either later today or tomorrow morning.
On Fri, Nov 20, 2020 at 8:43 AM Wakahisa <no...@github.com> wrote:
> @nevi-me <https://github.com/nevi-me> requested your review on: #8545
> <https://github.com/apache/arrow/pull/8545> ARROW-9674
> <https://issues.apache.org/jira/browse/ARROW-9674>: [Rust] Make the
> parquet read and writers Send.
>
> —
> You are receiving this because your review was requested.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/arrow/pull/8545#event-4020953257>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AADXZMJK2ZRMBDA55ZEYF73SQZXBBANCNFSM4TCHH24A>
> .
>
----------------------------------------------------------------
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] alamb commented on pull request #8545: ARROW-9674: [Rust] Make the parquet read and writers Send
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #8545:
URL: https://github.com/apache/arrow/pull/8545#issuecomment-731468020
> Beat me to the merge @alamb by seconds
🏃 Sorry @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] nevi-me commented on pull request #8545: ARROW-9674: [Rust] Make the parquet read and writers Send
Posted by GitBox <gi...@apache.org>.
nevi-me commented on pull request #8545:
URL: https://github.com/apache/arrow/pull/8545#issuecomment-723442063
@Marwes may you please rebase this, I think the conflicts are from your other PR, which might share some commits with this one
----------------------------------------------------------------
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] Marwes commented on pull request #8545: ARROW-9674: [Rust] Make the parquet read and writers Send
Posted by GitBox <gi...@apache.org>.
Marwes commented on pull request #8545:
URL: https://github.com/apache/arrow/pull/8545#issuecomment-731113355
The `Cell` needs to be removed to make the buffer types `Send`. Anyway, it is fixed.
----------------------------------------------------------------
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