Dan Hecht has posted comments on this change.
Change subject: IMPALA-5220: memory maintenance cleanup
......................................................................
Patch Set 6:
(2 comments)
With this change, we're globally relying on tc-malloc aggressive memory
decommit, right? So wouldn't it make sense to put that check you had into
Init()?
MallocExtension::instance()->GetNumericProperty(
"tcmalloc.aggressive_memory_decommit", &aggressive_decommit_enabled);
CHECK_EQ(true, aggressive_decommit_enabled);
http://gerrit.cloudera.org:8080/#/c/6626/6/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:
Line 300: if (consumption_metric_ != NULL)
consumption_->Set(consumption_metric_->value());
how about using RefreshConsumptionFromMetric() here? using the same code would
make it easier to understand where we do this sync.
http://gerrit.cloudera.org:8080/#/c/6626/6/be/src/runtime/mem-tracker.h
File be/src/runtime/mem-tracker.h:
Line 200: consumption_->Set(consumption_metric_->value());
how about using RefreshConsumptionFromMetric() here as well?
--
To view, visit http://gerrit.cloudera.org:8080/6626
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f822b294ab253d6f2828fc52f353aecaaf9b701
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Reviewer: anujphadke <[email protected]>
Gerrit-HasComments: Yes
