harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mikhail Fursov" <mike.fur...@gmail.com>
Subject Re: [drlvm][em] Value-profiling implementation is avaible.
Date Tue, 31 Oct 2006 09:06:26 GMT
Yuri,
I support Egor in the most of the points. Your code is conceptually OK and
is fitted into EM in the right way, but you can't get your code commited if
you do not check it on Linux and Windows.

On 31 Oct 2006 14:35:02 +0600, Egor Pasko <egor.pasko@gmail.com> wrote:
>
> * you do one global lock (profilesLock), that would be a *real crawl* :)
>   Profiling is method-wise, I'd suggest a lock per method. How is
>   that?

Agree with Egor.
In the edge profiler I use only one global lock to keep profiles collection
safe when adding new profiles.
So the one global lock is OK in this case. But as I see Yuri uses the lock
even for insertion of new values. Do you really need this lock here?

* do you have an .emprofile to test your implementation? Would have
>   been great!
>
> * why EM_PCTYPE updated, but ProfileType left as-is? is it intentional?


ProfileType is a JIT wrapper for EM types. Yuri's patch does not contain any
JIT changes. So this is  a TODO: add value profiling support to the JIT.

* why a value is a mere 'int'? what about pointer profiling? it is different
>   on IA-32 and x86_64. How would you write a portable piece of
>   profiler? Not nerdy, just curious :)


Reasonable question. I think value profiler should not care(at least the
first version) about the semantics of data it collects. int32 is enough to
profile type information for methods. int64 sized values could be added
latter.

-- 
Mikhail Fursov

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message