<div class="gmail_quote">On Fri, Jul 6, 2012 at 3:40 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_extra"><div class="gmail_quote"><div class="im">On Fri, Jul 6, 2012 at 9:10 AM, Jakob Stoklund Olesen <span dir="ltr"><<a href="mailto:stoklund@2pi.dk" target="_blank">stoklund@2pi.dk</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><br>
On Jul 6, 2012, at 1:28 AM, Chandler Carruth <<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>> wrote:<br>
<br>
> This patch adds what seems to me to be a very efficient merge operation for LiveInterval to address the quadratic behavior seen in PR12652 when merging extremely large, extremely distant disjoint intervals created by ASan's strange code layout.<br>


<br>
</div>Thanks for fixing this, Chandler.<br>
<br>
It's good to get rid of the quadratic algorithm, but I am a bit concerned about the typical case because you are a) copying the whole thing on every call, and b) calling malloc/free at least once per call.<br>
<br>
Could you measure the compile time impact on something mundane, like compiling clang, please?<br></blockquote><div><br></div></div><div>Yea, I'll dig a bit into the common case.</div><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


<div><br>
> There is one other user of addRangeFrom left (join) and I think I can refactor this code a bit and share the merge logic for that routine as well. But I didn't want to do that in this patch.<br>
<br>
</div>I am not sure that join() should use the same algorithm. It gets called a lot with a very small RHS interval that merges into the existing large LHS without shifting things around. Your algorithm would turn O(logN) into O(N) in that case. (Plus the malloc thing).<br>


<br>
To exploit this common case (LHS large, RHS tiny, LHS.size() doesn't change), you could:<br></blockquote><div><br></div></div><div>Ah, I didn't realize that size not changing was the common case! It makes sense though. If size had been changing each time, just building a new vector doesn't seem so bad.</div>
<div class="im">
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
1. Start with a binary search in case RHS starts in the middle of a huge LHS.<br>
<br>
2. Merge directly into LHS whenever possible, use NewRanges only as an overflow area.<br>
<br>
3. Grow LHS to the required size, and going backwards, merge NewRanges into LHS.<br></blockquote><div><br></div></div><div>Yea, I'll look at this (or at least something quite similar) so that we do as much as we can to leave the LHS in place when it doesn't need to grow.</div>

<div><br></div><div>I'll integrate this (as wel as your and Ben's comments) into a fresh patch shortly for you to look at before I commit.</div></div></div></blockquote><div><br></div><div><br></div><div>This isn't nearly finished, and its still missing both some cleanups I suspect and it has a few bugs that I haven't had time to track down, but wanted to see if this is more along the lines you were thinking of algorithm-wise. New patch attached.</div>
<div><br></div><div>If (algorithmically) you're liking this direction, then after cleaning up, fixing bugs, and testing performance in both cases, is this a good initial step to commit? Not precluding any further work to make join() use the same logic, and ways of factoring the value number manipulation out of the merge logic....</div>
</div>