From: <jen...@de...> - 2007-08-17 17:07:33
|
Hello, here is an update for my Oprofile JIT support patch set. Only the the first four of the set were changed Patches 1-3 libopagent.patch opjitconv.patch opcontrol.patch: - Fix: JIT-Dump datastructures need to be 32/64 bit independent (struct timeval has 64 bit fields on 64 bit systems) - Fix: ELF target. Whether it needs to be ELF32 or ELF64 is set correctly in the JIT-Dump - Enhancement: opcontrol and opjitconv are modified to take the life time of a symbol into account when overlaps are detected (overlaps result in address space reusage of the JIT). In this case there is a heuristic now which selects the symbol with the longest lifetime and drops the other symbols that overlap with it. To signal what is going on here, the "winning" symbol becomes a %n added, where n is a percent value from the total life time. Patches 4 jvmti_agent: - I discovered that picked up an older version of the code for CVS head adaption. The previously posted one has debug code and style issues. Sorry. Best, Jens -- Jens Wilke Linux on System z - Application Development Tools phone +49-(0)7031-16-3936 - tl *120-3936 - email jen...@de... IBM Germany Lab, Schoenaicher Str. 220, 71032 Boeblingen |
From: Maynard J. <may...@us...> - 2007-08-22 22:40:34
Attachments:
oprof-jit-bitness-fixes.patch
|
jen...@de... wrote: > Hello, > > here is an update for my Oprofile JIT support patch set. > [snip] Jens, I built oprofile with your latest patches. Generally, they seem fine, and I found no regressions compared to the previous patch set. However, I've been banging away a little harder on this stuff this week and found some cases where things break. I've started to investigate how the agent lib and libopagent would work in a bi-arch environment. So I began with 32-bit JIT-enabled oprofile tools trying to profile a 64-bit JVM. In the process, I discovered an old bug that prevented _any_ 64-bit anonymous samples from being recorded properly by 32-bit oprofile. I posted a separate patch (fixes daemon/opd_anon.c) to the mailing list for this problem, since this was a pre-existing bug in OProfile. But even with the opd_anon.c patch, I still wasn't getting anonymous samples in 'opreport -l' for a 64-bit JVM with 32-bit oprofile. However, 'opreport' (with no options) did show the summary of the anonymous samples in the output. The attached patch addresses the problem exhibited with 'opreport -l'. I also had to make some changes in callgraph_container.cpp in this patch since the --callgraph option was not giving correct output. Basically, I'm just skipping the processing of any op_bfd's that are for anonymous samples. I think this is fallout from switching over to having the start addr always being 0 for anonymous mappings. You'll also notice that I modified your algorithm for deciding whether or not an op_bfd represented an anonymous mapping, since this resulted in missing data in the report for anonymous samples not backed by a JIT dump file. Of course, you know this code way better than I do, so you may find my hacks to fix these issues are not very elegant. Feel free to make changes at will. P.S. Regards, -Maynard > Best, > > Jens > > -- > Jens Wilke > Linux on System z - Application Development Tools > phone +49-(0)7031-16-3936 - tl *120-3936 - email jen...@de... > IBM Germany Lab, Schoenaicher Str. 220, 71032 Boeblingen > > > ------------------------------------------------------------------------- > This SF.net email is sponsored by: Splunk Inc. > Still grepping through log files to find problems? Stop. > Now Search log events and configuration files using AJAX and a browser. > Download your FREE copy of Splunk now >> http://get.splunk.com/ > _______________________________________________ > oprofile-list mailing list > opr...@li... > https://lists.sourceforge.net/lists/listinfo/oprofile-list |
From: Jens W. <jen...@de...> - 2007-08-24 15:44:45
|
On Thursday 23 August 2007, Maynard Johnson wrote: > I built oprofile with your latest patches. Generally, they seem fine, > and I found no regressions compared to the previous patch set. However, > I've been banging away a little harder on this stuff this week and > found some cases where things break. Great! (not the breaking but the banging) > But even with the opd_anon.c patch, I still wasn't getting anonymous > samples in 'opreport -l' for a 64-bit JVM with 32-bit oprofile. > However, 'opreport' (with no options) did show the summary of the > anonymous samples in the output. The attached patch addresses the > problem exhibited with 'opreport -l'. > > I also had to make some changes in callgraph_container.cpp in this patch > since the --callgraph option was not giving correct output. Basically, > I'm just skipping the processing of any op_bfd's that are for anonymous > samples. I think this is fallout from switching over to having the > start addr always being 0 for anonymous mappings. > > You'll also notice that I modified your algorithm for deciding whether > or not an op_bfd represented an anonymous mapping, since this resulted > in missing data in the report for anonymous samples not backed by a JIT > dump file. > > Of course, you know this code way better than I do, so you may find my > hacks to fix these issues are not very elegant. Feel free to make > changes at will. Basically your patches look good, I think you discoverred the points of the rough edges of my code. In detail: --snip-- @@ -156,6 +153,16 @@ op_bfd::op_bfd(string const & archive, s out: add_symbols(symbols, symbol_filter); + if (!syms.empty()) + sym_name = syms[0].name(); + + if (!sym_name.empty() && + ((sym_name.find("?anon", 0) == 0) || + (filename.rfind(suf) == filename.size() - suf.size()))) + anonobj = true; + else + anonobj = false; + return; --snip-- Basically I don't like the decision logic I have implemented here at all. My orignial code decides on the file extension of the ELF file. I think we need to check whether the file is in "/var/lib/oprofile" also, and belongs really to the ones we created. OTOH there is perhaps a more elegant approach here I didn't think of yet. I don't understand completely what you have added here. If the first symbol name is empty it is not an anonobj? Can we simplify this like: --snip-- + if (!syms.empty() && + ((syms[0].name.find("?anon", 0) == 0) || + (filename.rfind(suf) == filename.size() - suf.size()))) + anonobj = true; + else + anonobj = false; --snip-- The other idea that comes to my mind when I am looking at this is, whether it would make more sense to remove the decision on the filename in favor of something that is within the ELF. By example (not totally elegant) the first symbol could be named "Oprofile JIT-Dump". --snip-- void op_bfd::get_symbol_range(symbol_index_t sym_idx, - unsigned long & start, unsigned long & end) const + bfd_vma & start, bfd_vma & end) const { op_bfd_symbol const & sym = syms[sym_idx]; bool const verbose = cverb << (vbfd & vlevel1); - + end = 0; if (anonobj) { start = sym.vma(); + /* For anonymous mappings not backed by a JIT dump file, + * there's only one symbol in the op_bfd, so all + * samples will be attributed to that symbol; thus, + * start and end can be assigned as follows. + */ + std::string sym_name = syms[sym_idx].name(); + + if (!sym_name.empty() && (sym_name.find("?anon", 0) == 0)) { + if (sizeof(bfd_vma)==8) + end = 0xffffffffffffffffll; + else + end = 0xffffffff; + } + cverb << (vbfd & vlevel1) << "This is anonobj: "; --snip-- Oops. I don't think it is good to have that much special cases in get_symbol_range. Maybe we can push this to the Symbol implementation, and, do it at when the Symbol gets constructed. Anyways, thanks for finding all this out! I am away for two weeks now and will look into this deeply after it. If there is more to do pls. collect and post as munch issues as you can meanwhile. Best, Jens -- Jens Wilke Linux on System z - Application Development Tools phone +49-(0)7031-16-3936 - tl *120-3936 - email jen...@de... IBM Germany Lab, Schoenaicher Str. 220, 71032 Boeblingen |