You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/07/21 10:08:01 UTC
[GitHub] [incubator-doris] vagetablechicken opened a new pull request #4135: [MemTracker] make all MemTrackers shared
vagetablechicken opened a new pull request #4135:
URL: https://github.com/apache/incubator-doris/pull/4135
Ref https://github.com/apache/incubator-doris/issues/3714
We make all MemTrackers shared.
As follows:
1. all MemTracker raw ptr -> shared_ptr
2. Use CreateTracker() to create new MemTracker(in order to add itself to its parent)
3. RowBatch & MemPool still use raw ptrs of MemTracker, it's esay to ensure RowBatch & MemPool dtor exec before MemTracker's dtor. So we don't change these code.
4. MemTracker can use RumtimeProfile's counter to calc consumption. So RuntimeProfile's counter need to be shared too. We add a shared counter pool to store the shared counter, don't change other counters of RuntimeProfile.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] vagetablechicken commented on pull request #4135: [MemTracker] make all MemTrackers shared
Posted by GitBox <gi...@apache.org>.
vagetablechicken commented on pull request #4135:
URL: https://github.com/apache/incubator-doris/pull/4135#issuecomment-662351521
> Hello, I have two questions to ask。
>
> 1. shared ptr use for avoid memory leaks. but at the same time, it brings about several problems：
> (1）There are additional costs of ref count of ptr
> (2) The interface becomes infectious
> So Is it good enough to do this replacement of shared_ptr ? what benefit us?
> 2. `unique_ptr` runtime expenses is close to the raw pointer and it's simple enough to use。
> In most cases, should we replacing it with `shared_prt`.
As metioned in the first comment, this pr is for showing all MemTracker on BE's website, like kudu. So we need to make all MemTrackers not owned by anyone. If we get it `on web`, it shouldn't be destoryed with the obj which created it.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] HappenLee commented on pull request #4135: [MemTracker] make all MemTrackers shared
Posted by GitBox <gi...@apache.org>.
HappenLee commented on pull request #4135:
URL: https://github.com/apache/incubator-doris/pull/4135#issuecomment-662322631
Hello, I have two questions to ask。
1. shared ptr use for avoid memory leaks. but at the same time, it brings about several problems：
(1）There are additional costs of ref count of ptr
(2) The interface becomes infectious
So Is it good enough to do this replacement of shared_ptr ? what benefit us?
2. ```unique_ptr``` runtime expenses is close to the raw pointer and it's simple enough to use。
In most cases, should we replacing it with ```shared_prt```.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] vagetablechicken edited a comment on pull request #4135: [MemTracker] make all MemTrackers shared
Posted by GitBox <gi...@apache.org>.
vagetablechicken edited a comment on pull request #4135:
URL: https://github.com/apache/incubator-doris/pull/4135#issuecomment-662452697
> So the mainly reason use `shared_prt ` is only to show MemTracker on website?
Yes. We had met a lot of problems of memory, they usually are accompanied with large MemPool consumption. But we can't figure out, MemPool consumption is the total metric. I think showing MemTracker is valuable.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] HappenLee commented on pull request #4135: [MemTracker] make all MemTrackers shared
Posted by GitBox <gi...@apache.org>.
HappenLee commented on pull request #4135:
URL: https://github.com/apache/incubator-doris/pull/4135#issuecomment-662519072
> > Hello, I have two questions to ask。
> >
> > 1. shared ptr use for avoid memory leaks. but at the same time, it brings about several problems：
> > (1）There are additional costs of ref count of ptr
> > (2) The interface becomes infectious
> > So Is it good enough to do this replacement of shared_ptr ? what benefit us?
> > 2. `unique_ptr` runtime expenses is close to the raw pointer and it's simple enough to use。
> > In most cases, should we replacing it with `shared_prt`.
>
> As metioned in the first comment, this pr is for showing all MemTracker on BE's website #3714 , like kudu. So we need to make all MemTrackers not owned by anyone. If we get it `on web`, it shouldn't be destoryed with the obj which created it.
The lifecycle of MemTracker for a query is clear。Now RunningProfile don't use shared ptr, also can show running statistics of query. So it 's really necessary to shared_ptr to manage lifecycles？Can it be a tree structure like the runningprofile？
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] kangkaisen commented on pull request #4135: [MemTracker] make all MemTrackers shared
Posted by GitBox <gi...@apache.org>.
kangkaisen commented on pull request #4135:
URL: https://github.com/apache/incubator-doris/pull/4135#issuecomment-662418107
> > Hello, I have two questions to ask。
> >
> > 1. shared ptr use for avoid memory leaks. but at the same time, it brings about several problems：
> > (1）There are additional costs of ref count of ptr
> > (2) The interface becomes infectious
> > So Is it good enough to do this replacement of shared_ptr ? what benefit us?
> > 2. `unique_ptr` runtime expenses is close to the raw pointer and it's simple enough to use。
> > In most cases, should we replacing it with `shared_prt`.
>
> As metioned in the first comment, this pr is for showing all MemTracker on BE's website #3714 , like kudu. So we need to make all MemTrackers not owned by anyone. If we get it `on web`, it shouldn't be destoryed with the obj which created it.
So the mainly reason use `shared_prt ` is only to show MemTracker on website?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] vagetablechicken commented on pull request #4135: [MemTracker] make all MemTrackers shared
Posted by GitBox <gi...@apache.org>.
vagetablechicken commented on pull request #4135:
URL: https://github.com/apache/incubator-doris/pull/4135#issuecomment-662791622
> The lifecycle of MemTracker for a query is clear。Now RunningProfile don't use shared ptr, also can show running statistics of query. So it 's really necessary to shared_ptr to manage lifecycles？Can it be a tree structure like the runningprofile？
It's not for the lifecycle of MemTracker. How do you know the exact mem consumption of one query? What if the MemTracker node destoryed, when we are traversing the tree?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] vagetablechicken commented on pull request #4135: [MemTracker] make all MemTrackers shared
Posted by GitBox <gi...@apache.org>.
vagetablechicken commented on pull request #4135:
URL: https://github.com/apache/incubator-doris/pull/4135#issuecomment-662452697
> So the mainly reason use `shared_prt ` is only to show MemTracker on website?
Yes. We had met a lot of problems of memory, they usually are accompanied with large MemPool consumption. But we can't figure out, MemPool consumption is the total metric. I think showing MemTracker is valuable.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] chaoyli merged pull request #4135: [MemTracker] make all MemTrackers shared
Posted by GitBox <gi...@apache.org>.
chaoyli merged pull request #4135:
URL: https://github.com/apache/incubator-doris/pull/4135
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] vagetablechicken edited a comment on pull request #4135: [MemTracker] make all MemTrackers shared
Posted by GitBox <gi...@apache.org>.
vagetablechicken edited a comment on pull request #4135:
URL: https://github.com/apache/incubator-doris/pull/4135#issuecomment-662351521
> Hello, I have two questions to ask。
>
> 1. shared ptr use for avoid memory leaks. but at the same time, it brings about several problems：
> (1）There are additional costs of ref count of ptr
> (2) The interface becomes infectious
> So Is it good enough to do this replacement of shared_ptr ? what benefit us?
> 2. `unique_ptr` runtime expenses is close to the raw pointer and it's simple enough to use。
> In most cases, should we replacing it with `shared_prt`.
As metioned in the first comment, this pr is for showing all MemTracker on BE's website #3714 , like kudu. So we need to make all MemTrackers not owned by anyone. If we get it `on web`, it shouldn't be destoryed with the obj which created it.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org