[PATCH] D26967: Put opt-viewer critical items in parallel

[Brian Cain via Phabricator via llvm-commits]

bcain added a comment.

In https://reviews.llvm.org/D26967#651691, @anemet wrote:

> Thanks for the update, Brian:
>
> In https://reviews.llvm.org/D26967#650414, @bcain wrote:
>
> > In https://reviews.llvm.org/D26967#617705, @anemet wrote:
> >
> > > I just sent you an email with the details.
> >
> >
> > The differences are limited to the index.html.  Most of the differences are inconsequential.  I'm trying to explain them one by one and a few more unexplained ones remain, so stay tuned.
>
>
> There were differences in the source views as well.  I just investigated and looks like the remark uniquing is working differently between the serial and parallel -- neither of them being correct.


Oh, yes.  I suppose I won't be able to produce most of the source views correctly for this example unless I have all of the source files (and have them installed at the same paths as the producer).

> When we're uniquing the remarks using all_remarks, key does not include the containing function, so if there is no hotness difference between the different functions (inlining contexts) they will get uniqued.
> 
> On the other hand, the parallel version only does uniquing within one thread.  We do merge all_remarks from each thread but file_remarks at that point could contain duplicates.  We should probably unique in merge_dicts?

file_remarks is indexed on file-line.  The difference that I can see between serial and parallel implementation is not the presence/absence of duplicates but the order in which they're append()ed is not the same.  But that shouldn't matter if file_remarks are only referenced while generating the source file renders, right?

If the .key property of Remark would be better off by having a self.Function element, should I just add it?

Also: note that this parallel implementation spawns processes, not threads.  This means that globally visible changes like e.g. class-members have to be manually delivered back to the parent.  This is why the Remark.max_hotness is not updated in get_remarks().   Changing this implementation to be multithreaded is simple and only requires slightly different imports.

> If you want to see an example of this, diff _Users_adam_proj_org_llvm_build-rel_bin_.._include_c++_v1___functional_base.html between html-orig and html-par from the tarball I sent you.   On the very first line we annotate (line 63), there are licm remarks with different inlining contexts on the parallel version but not on the serial one.

I'll see if I can make a minimal reproducer, sorting through all of these differences is difficult to reason about IMO.


Repository:
  rL LLVM

https://reviews.llvm.org/D26967