[PATCH] D20408: [profile] entry eviction support in value profiler

[Xinliang David Li via llvm-commits]

On Thu, May 19, 2016 at 10:49 AM, Rong Xu <xur at google.com> wrote:

> xur added a comment.
>
> The eviction method is good. My concerns is we don't using lock in this
> function, there are chances of corrupted data in threaded programs. This
> eviction mechanism seems to increase the chances. And previously the
> corruption will only be count values. Now it could be targets.




> I would suggest to use atomic in setting the target when replacing.
>

 Target setting is just a store -- so it should be fine.

>
> Another related question: The counter allocation uses atomic operation.
> Why not the counter updates also using atomic?
>

The consideration is different. For counter update, not using atomic only
leads to small precision loss, but for allocation, it is important to use
atomic update to avoid leaks/corrupted data structure.   Another reason is
that counter update is much more frequent than counter allocation so the
choice also has  performance reason.

David


>
> -Rong
>
>
> http://reviews.llvm.org/D20408
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160519/56b599a3/attachment.html>