<div dir="ltr"><div>Hi Enrico,</div><div><br></div><div>> However, a HistoryThreads hardly seems to fit in [ValueImpl]</div><div>> I am assuming this is mostly because:</div><div>> - this is a pointer-based lookup, so it makes little sense on the ValueObject proper</div>
<div>> - but you still want to store it away somewhere once you computed it</div><div><br></div><div>Exactly.</div><div><br></div><div>> I admit not having followed through your previous patch in depth, so take my</div>
<div>> suggestion with a grain of salt.. couldn’t your MemoryHistory plugin handle</div><div>> the caching? i.e. all I would ever do when asked for a HistoryThreadsSP is</div><div>> plugin->GetHistoryThreads(pointer_value); and the plugin would internally</div>
<div>> handle a per-process cache of pointer_value —> thread list</div><div><br></div><div>I would also like to be able to "renew" that thread list, so you don't get stuck with one list that is no longer correct. The SBValue API did the caching and you could always create a new SBValue to "renew".</div>
<div><br></div><div>Although I think you're right, a generic "here's a pointer, give me a thread list" sounds reasonable. The issue with that is that SB doesn't know about a thread list. Mostly because even internally, ThreadList is not really just a container for Thread objects, but has a lot of process-related logic.</div>
<div><br></div><div>What would you suggest? Having a SBThreadList that would be just a container (and thus would differ from ThreadList)?</div><div><br></div><div>Kuba</div><div><br></div></div><div class="gmail_extra"><br>
<br><div class="gmail_quote">On Wed, Sep 3, 2014 at 2:54 PM, Enrico Granata <span dir="ltr"><<a href="mailto:egranata@apple.com" target="_blank">egranata@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word"><div>I think I mentioned that I was not sure SBValue was the best place to place this functionality (it’s address based, not ValueObject based), but it looked like others on the team felt differently, so no big deal there</div>
<div><br></div><div>One thing that worries me a little bit more is how the history threads are being stored on the ValueImpl - I am not sure I agree with that</div><div><br></div><div>The ValueImpl is essentially a way to reconstruct the underlying ValueObject for an SBValue given choices on dynamic/synthetic values; I could see us adding more axes to that class if we ended up with more features that could potentially generate different “views” of the same underlying root ValueObject</div>
<div><br></div><div>However, a HistoryThreads hardly seems to fit in there - I am assuming this is mostly because:</div><div>- this is a pointer-based lookup, so it makes little sense on the ValueObject proper</div><div>- but you still want to store it away somewhere once you computed it</div>
<div><br></div><div>I admit not having followed through your previous patch in depth, so take my suggestion with a grain of salt.. couldn’t your MemoryHistory plugin handle the caching? i.e. all I would ever do when asked for a HistoryThreadsSP is plugin->GetHistoryThreads(pointer_value); and the plugin would internally handle a per-process cache of pointer_value —> thread list</div>
<div><br></div><div>The advantage is that now the question could be asked from anywhere; I could ask it of an internal ValueObject, or of an SBValue, and the right thing would happen consistently (w.r.t. caching and whatnot) because now the plugin is handling this in a sane manner</div>
<div><br></div><div>This last one is minor but,</div><div><div>+typedef std::shared_ptr<HistoryThreads> HistoryThreadsSP;</div><div>+</div></div><div><br></div><div>why do you need to define this in SBValue.cpp?</div>
<div>Can’t HistoryThreadsSP be defined with all the other FooSP typedefs, so it’s visible everywhere?</div><br><div><blockquote type="cite"><div class=""><div>On Sep 3, 2014, at 11:41 AM, Kuba Brecka <<a href="mailto:kuba.brecka@gmail.com" target="_blank">kuba.brecka@gmail.com</a>> wrote:</div>
<br></div><div><div><div class="h5">This patch depends on <a href="http://reviews.llvm.org/D4596" target="_blank">http://reviews.llvm.org/D4596</a>.<br><br>As a continuation of the previous patch that adds a MemoryHistory plugin and implementation for ASan-provided malloc/free stack traces, this patch exposes this into the SB API. In short, these two new methods are added into SBValue:<br>
<br>* uint32_t SBValue::GetNumMemoryHistoryThreads ();<br>* SBThread SBValue::GetMemoryHistoryThreadAtIndex (uint32_t idx);<br><br>This corresponds to how we provide objects for which we don't have containers (SBFrame and GetNumFrames + GetFrameAtIndex). Note that exposing ThreadList into a generic SBThreadList container would not be straightforward, because currently ThreadList is not a generic container of threads, but instead holds functionality tied to a process and can currently only be used to hold all threads in a process.<br>
<br><a href="http://reviews.llvm.org/D5175" target="_blank">http://reviews.llvm.org/D5175</a><br><br>Files:<br> include/lldb/API/SBValue.h<br> scripts/Python/interface/SBValue.i<br> source/API/SBValue.cpp<br> test/functionalities/asan/TestAsan.py<br>
</div></div><div class=""><span><D5175.13219.patch></span>_______________________________________________<br>lldb-commits mailing list<br><a href="mailto:lldb-commits@cs.uiuc.edu" target="_blank">lldb-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br></div></div></blockquote></div><br><div>
<div style="color:rgb(0,0,0);letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word"><div style="color:rgb(0,0,0);letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word">
<div style="color:rgb(0,0,0);letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word"><div style="color:rgb(0,0,0);letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word">
<div style="color:rgb(0,0,0);letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word"><div>Thanks,</div><div><i>- Enrico</i><br>📩 egranata@<font color="#ff2600"></font>.com ☎️ 27683</div>
<div><br></div></div></div></div></div></div><br><br>
</div>
<br></div></blockquote></div><br></div>