<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">Hi Yaron,<br>
On 08/04/2014 10:31 AM, Yaron Keren wrote:<br>
</div>
<blockquote
cite="mid:CANa4zJqvFcFB0cdLjvhdJiwiFvdX1y0xifxD+C+ciTSn61Fxmg@mail.gmail.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
<div dir="rtl">
<div dir="ltr">Hi Vassil,</div>
<div dir="ltr"><br>
</div>
<div dir="ltr">If you decide to keep the last code posted as a
cling patch, it could do with 'I' only instead of 'I' and
'current', and when MI is the first node the code should set
MIChainHead but not set its Next.</div>
</div>
</blockquote>
Thanks for pointing out, will do.<br>
<blockquote
cite="mid:CANa4zJqvFcFB0cdLjvhdJiwiFvdX1y0xifxD+C+ciTSn61Fxmg@mail.gmail.com"
type="cite">
<div dir="rtl">
<div dir="ltr"><br>
</div>
<div dir="ltr">To the point, <span
style="font-family:arial,sans-serif;font-size:12.727272033691406px">ReleaseMacroInfo
just releases the SmallVector Tokens memory if it wasn't
small.</span></div>
<div dir="ltr"><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px">It
did not modify anything else</span><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px">.
You could still </span><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px">removeMacro</span><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px"> without </span><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px">ReleaseMacroInfo.</span></div>
</div>
</blockquote>
Thanks for explaining. My code looks like this:<br>
<br>
void Preprocessor::removeMacro(IdentifierInfo *II, const
MacroDirective *MD) {<br>
assert(II && MD);<br>
assert(!MD->getPrevious() && "Already attached to a
MacroDirective history.");<br>
<br>
//Release the MacroInfo allocated space so it can be reused.<br>
MacroInfo* MI = MD->getMacroInfo();<br>
if (MI) {<br>
ReleaseMacroInfo(MI);<br>
}<br>
Macros.erase(II);<br>
}<br>
<br>
IIUC I need to check if the small vector isSmall and if not then do
a ReleaseMacro, or even this is redundant?<br>
<blockquote
cite="mid:CANa4zJqvFcFB0cdLjvhdJiwiFvdX1y0xifxD+C+ciTSn61Fxmg@mail.gmail.com"
type="cite">
<div dir="rtl">
<div dir="ltr"><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br>
</span></div>
<div dir="ltr"><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px">There's
lots of places in clang where memory is allocated and not
released until destruction for performance. </span></div>
<div dir="ltr"><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px">The
whole AST for starters... <br>
</span></div>
</div>
</blockquote>
<blockquote
cite="mid:CANa4zJqvFcFB0cdLjvhdJiwiFvdX1y0xifxD+C+ciTSn61Fxmg@mail.gmail.com"
type="cite">
<div dir="rtl">
<div dir="ltr"><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br>
</span></div>
<div dir="ltr"><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px">It
would be nice to early release the Tokens but </span><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px">In
this context it would hardly move the needle.</span></div>
</div>
</blockquote>
I agree. So I need to somehow implement it.<br>
<blockquote
cite="mid:CANa4zJqvFcFB0cdLjvhdJiwiFvdX1y0xifxD+C+ciTSn61Fxmg@mail.gmail.com"
type="cite">
<div dir="rtl">
<div dir="ltr"><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br>
</span></div>
<div dir="ltr"><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px">cling
memory use should going up every iteration due to this
startegy, no?</span></div>
</div>
</blockquote>
Yes, it grows. The context I want things removed is support of 'code
unloading'. Say:<br>
[cling] #include "MyFile.h"<br>
[cling] MyClass m; m.do();<br>
// Figure out that do is not what I want. I edit the file and do:<br>
[cling] #include "MyFile.h" // It would undo everything up to
#include "MyFile.h" (inclusively). I want the memory to be reduced
also. This is why I need to delete the macros and not only undef
them. (The same holds for the AST)<br>
[cling] MyClass m; m.do(); // Here do and MyClass may have
completely different implementation.<br>
<br>
Vassil<br>
<blockquote
cite="mid:CANa4zJqvFcFB0cdLjvhdJiwiFvdX1y0xifxD+C+ciTSn61Fxmg@mail.gmail.com"
type="cite">
<div dir="rtl">
<div dir="ltr"><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br>
</span></div>
<div dir="ltr"><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px">Yar</span><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px">on</span></div>
<div dir="ltr"><br>
</div>
<div dir="ltr"><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br>
</span></div>
<div dir="ltr"><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br>
</span></div>
</div>
<div class="gmail_extra"><br>
<br>
<div class="gmail_quote">
<div dir="ltr">2014-08-04 10:47 GMT+03:00 Vassil Vassilev <span
dir="ltr"><<a moz-do-not-send="true"
href="mailto:vasil.georgiev.vasilev@cern.ch"
target="_blank">vasil.georgiev.vasilev@cern.ch</a>></span>:</div>
<blockquote class="gmail_quote" style="margin:0
.8ex;border-left:1px #ccc solid;border-right:1px #ccc
solid;padding-left:1ex;padding-right:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<div>Hi Richard,<br>
Thanks for the fix!<br>
<br>
Unfortunately it doesn't help for cling case. I
implement a removeMacro routine using ReleaseMacroInfo.
ReleaseMacroInfo allowed me to implement efficiently the
removal of a macro instead of dragging a long def undef
chains, for example.<br>
IIUC it allowed some memory reduction in some cases
for clang, too. Is there any chance to keep the
ReleaseMacroInfo upstream? <br>
<span class="HOEnZb"><font color="#888888"> Vassil</font></span>
<div>
<div class="h5"><br>
On 08/04/2014 01:50 AM, Richard Smith wrote:<br>
</div>
</div>
</div>
<div>
<div class="h5">
<blockquote type="cite">
<div dir="ltr">Fixed in a much more simple way in r<span
style="color:rgb(0,0,0)">214675. Thanks for
reporting!</span></div>
<div class="gmail_extra"><br>
<br>
<div class="gmail_quote">On Sun, Aug 3, 2014 at
11:52 AM, Vassil Vassilev <span dir="ltr"><<a
moz-do-not-send="true"
href="mailto:vvasilev@cern.ch"
target="_blank">vvasilev@cern.ch</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0
0 0 .8ex;border-left:1px #ccc
solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<div>I will try just one more time and then
shut up :)
<div><br>
<br>
diff --git a/lib/Lex/PPDirectives.cpp
b/lib/Lex/PPDirectives.cpp<br>
index 5f38387..000ea7a 100644<br>
--- a/lib/Lex/PPDirectives.cpp<br>
+++ b/lib/Lex/PPDirectives.cpp<br>
@@ -94,6 +94,19 @@
Preprocessor::AllocateVisibilityMacroDirective(SourceLocation
Loc,<br>
/// error in the macro definition.<br>
void
Preprocessor::ReleaseMacroInfo(MacroInfo
*MI) {<br>
// Don't try to reuse the storage;
this only happens on error paths.<br>
+<br>
+ // If this is on the macro info
chain, avoid double deletion on
teardown.<br>
+ MacroInfoChain *current =
MIChainHead;<br>
+ while (MacroInfoChain *I = current) {<br>
+ if (&(I->MI) == MI) {<br>
+ I->Next = (I->Next) ?
I->Next->Next : 0;<br>
+ if (I == MIChainHead)<br>
</div>
+ MIChainHead = I->Next;
<div><br>
+ break;<br>
+ }<br>
+ current = I->Next;<br>
+ }<br>
+<br>
MI->~MacroInfo();<br>
}<br>
<br>
<br>
</div>
<div>
<div> On 03/08/14 20:47, Vassil Vassilev
wrote:<br>
</div>
</div>
</div>
<div>
<div>
<blockquote type="cite">
<div>Hi,<br>
Sorry overlooked, thanks for
pointing it out!<br>
I hope this is what we want.<br>
Vassil<br>
<br>
diff --git
a/lib/Lex/PPDirectives.cpp
b/lib/Lex/PPDirectives.cpp<br>
index 5f38387..000ea7a 100644<br>
--- a/lib/Lex/PPDirectives.cpp<br>
+++ b/lib/Lex/PPDirectives.cpp<br>
@@ -94,6 +94,19 @@
Preprocessor::AllocateVisibilityMacroDirective(SourceLocation
Loc,<br>
/// error in the macro definition.<br>
void
Preprocessor::ReleaseMacroInfo(MacroInfo
*MI) {<br>
// Don't try to reuse the
storage; this only happens on error
paths.<br>
+<br>
+ // If this is on the macro info
chain, avoid double deletion on
teardown.<br>
+ MacroInfoChain *current =
MIChainHead;<br>
+ while (MacroInfoChain *I =
current) {<br>
+ if (&(I->MI) == MI) {<br>
+ I->Next = (I->Next) ?
I->Next->Next : 0;<br>
+ if (I == MIChainHead)<br>
+ MIChainHead = I;<br>
+ break;<br>
+ }<br>
+ current = I->Next;<br>
+ }<br>
+<br>
MI->~MacroInfo();<br>
}<br>
<br>
On 03/08/14 20:28, Yaron Keren
wrote:<br>
</div>
<blockquote type="cite">
<div dir="rtl">
<div dir="ltr">Hi,</div>
<div dir="ltr"><br>
</div>
<div dir="ltr">MIChainHead is a
pointer to the head of a linked
list of MacroInfoChain nodes,
each containing a MacroInfo
and MacroInfoChain*.<br>
</div>
<div dir="ltr"> <br>
</div>
<div dir="ltr">Why does the while
loop modify MIChainHead on every
iteration?</div>
<div dir="ltr">MIChainHead should
be modified only if it points to
the node containing the removed <span
style="font-size:12.727272033691406px;font-family:arial,sans-serif">MacroInfo
MI. </span><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px">In
all other cases it should not
change.</span></div>
<div dir="ltr"><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px"><br>
</span></div>
<div dir="ltr">As it is now, the
loop will always terminate
with MIChainHead == nullptr.<br>
</div>
<div dir="ltr"><br>
</div>
<div dir="ltr">Yaron</div>
<div dir="ltr"><br>
</div>
<div class="gmail_extra"><br>
<br>
<div class="gmail_quote">
<div dir="ltr">2014-08-03
21:10 GMT+03:00 Vassil
Vassilev <span dir="ltr"><<a
moz-do-not-send="true"
href="mailto:vvasilev@cern.ch"
target="_blank">vvasilev@cern.ch</a>></span>:</div>
<blockquote
class="gmail_quote"
style="margin:0
.8ex;border-left:1px #ccc
solid;border-right:1px #ccc
solid;padding-left:1ex;padding-right:1ex">
<div bgcolor="#FFFFFF"
text="#000000">
<div>Hi Yaron,<br>
Yes I meant double
destruction.<span><font
color="#888888"><br>
Vassil</font></span>
<div>
<div><br>
On 03/08/14 20:08,
Yaron Keren wrote:<br>
</div>
</div>
</div>
<div>
<div>
<blockquote
type="cite">
<div dir="rtl">
<div dir="ltr">Hi
Vassil,</div>
<div dir="ltr"><br>
</div>
<div dir="ltr">Do
you mean double
destruction (not
deletion) of <span
style="font-family:arial,sans-serif;font-size:12.727272033691406px">MacroInfo
first time in </span><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px">ReleaseMacroInfo </span><span
style="font-family:arial,sans-serif;font-size:12.727272033691406px">and
the second
time in </span>~Preprocessor
via
~MacroInfoChain?</div>
<div dir="ltr"><br>
</div>
<div dir="ltr">
<div dir="ltr">
while
(MacroInfoChain
*I =
MIChainHead) {</div>
<div dir="ltr">
MIChainHead
= I->Next;</div>
<div dir="ltr">
I->~MacroInfoChain();</div>
<div dir="ltr">
}</div>
<div><br>
</div>
<div>or
something
else?</div>
<div><br>
</div>
<div>Yaron<br>
</div>
</div>
<div dir="ltr"><br>
</div>
</div>
<div
class="gmail_extra"><br>
<br>
<div
class="gmail_quote">
<div dir="ltr">2014-08-02
23:05
GMT+03:00
Vassil
Vassilev <span
dir="ltr"><<a
moz-do-not-send="true" href="mailto:vvasilev@cern.ch" target="_blank">vvasilev@cern.ch</a>></span>:</div>
<blockquote
class="gmail_quote"
style="margin:0
0 0
.8ex;border-left:1px
#ccc
solid;padding-left:1ex">Hi,<br>
In cases
where
ReleaseMacroInfo
gets called
and it doesn't
cleanup the
Preprocessor's
MIChainHead
can lead to
double
deletion. I am
sending the
patch that
fixes the
problem for
me.<br>
Vassil<br>
<br>
<br>
diff --git
a/lib/Lex/PPDirectives.cpp
b/lib/Lex/PPDirectives.cpp<br>
index
5f38387..1a9b5eb
100644<br>
---
a/lib/Lex/PPDirectives.cpp<br>
+++
b/lib/Lex/PPDirectives.cpp<br>
@@ -94,6
+94,14 @@
Preprocessor::AllocateVisibilityMacroDirective(SourceLocation
Loc,<br>
/// error in
the macro
definition.<br>
void
Preprocessor::ReleaseMacroInfo(MacroInfo
*MI) {<br>
// Don't
try to reuse
the storage;
this only
happens on
error paths.<br>
+<br>
+ // If this
is on the
macro info
chain, avoid
double
deletion on
teardown.<br>
+ while
(MacroInfoChain
*I =
MIChainHead) {<br>
+ if
(&(I->MI)
== MI)<br>
+
I->Next =
(I->Next) ?
I->Next->Next
: 0;<br>
+
MIChainHead =
I->Next;<br>
+ }<br>
+<br>
MI->~MacroInfo();<br>
}<br>
<br>
_______________________________________________<br>
cfe-commits
mailing list<br>
<a
moz-do-not-send="true"
href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a
moz-do-not-send="true"
href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits"
target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</div>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</blockquote>
<br>
<br>
<fieldset></fieldset>
<br>
<pre>_______________________________________________
cfe-commits mailing list
<a moz-do-not-send="true" href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a>
<a moz-do-not-send="true" href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a>
</pre>
</blockquote>
<br>
</div>
</div>
</div>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a moz-do-not-send="true"
href="mailto:cfe-commits@cs.uiuc.edu"
target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a moz-do-not-send="true"
href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits"
target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</div>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</body>
</html>