On the 0x213 day of Apache Harmony Mikhail Fursov wrote: > 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 <[EMAIL PROTECTED]> 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?
The lock is not necessary if you update a single word. In case of incrementing counters no synch is OK (minor inaccuracy), but here we are updating an std::map, which is dangerous. > * 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. OK, let's discuss it later -- Egor Pasko, Intel Managed Runtime Division
