Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/9282 )
Change subject: IMPALA-6116: Bound memory usage of DataStreamSevice's service queue ...................................................................... Patch Set 3: > (2 comments) > > > Patch Set 1: > > > > (6 comments) > > > > I went through the change and found that I would prefer to not > access dependencies through the ExecEnv singleton, but instead > inject them through the ctors and Init(). That way it feels easier > to me to follow the code. I don't feel strongly about it though. > > I'm curious how do others feel about this? > My feeling is that if something is a singleton, then it's usually easier for me to follow the code if it's accessed directly rather than adding indirection by passing around pointers to it (which ultimately I'll often need to trace back to figure out where the object came from anyway). Additionally, at least in some places, the architecture dictates some of these daemon services are singletons, so I don't see the advantage in pretending that they parametrizable. On the other hand, passing by argument can help show dependencies between objects. However, the dependencies are often just between the existence of the object but between the state of the bojects (e.g. whether Init() was called yet), so it doesn't always help. -- To view, visit http://gerrit.cloudera.org:8080/9282 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idea4262dfb0e0aa8d58ff6ea6a8aaaa248e880b9 Gerrit-Change-Number: 9282 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Thu, 15 Feb 2018 03:43:44 +0000 Gerrit-HasComments: No
