<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=Windows-1252">
</head>
<body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">
<br>
<div>
<div>On 01 Oct 2014, at 16:11, Stefan Karlsson <<a href="mailto:stefan.karlsson@oracle.com">stefan.karlsson@oracle.com</a>> wrote:</div>
<br class="Apple-interchange-newline">
<blockquote type="cite">
<div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">
<br>
On 2014-10-01 15:41, Erik Österlund wrote:<br>
<blockquote type="cite">Hey,<br>
<br>
I have to admit I'm not a huge fan of the current system for explicitly specializing closures (removing virtual calls).<br>
<br>
Here's a couple of problems I see with the current solution:<br>
1. The specialization macros are obviously not pretty<br>
2. It's awkward to have to remember to explicitly list the closure as specialized in the macros<br>
3. There are plenty of composite closure types out there. What I mean by this is when closures are combined, e.g. one closure is used to filter a memory range, and if passing the filter, it will invoke the actual closure, currently resulting in a virtual call
even though the composite structure is completely known at the call site.<br>
4. Each closure has to have like do_oop, do_oop_v, do_oop_nv, for both oop types and then a do_oop_work for joining them. Yuck! Asserts try to check that the _v and _nv methods do the same thing to combat programmer mistakes.<br>
<br>
With my alternative template magic solution:<br>
1. You won't have to explicitly specialize wanted closure types - they are automatically specialized unless the contrary is explicitly stated.<br>
2. Parameterized composite closure types can be used without unnecessary virtual call overheads.<br>
3. Only a single do_oop (do_metadata etc) member function is needed, and hence no need to put asserts trying to keep _v and _nv synchronized.<br>
4. It is backward compatible and does not require major refactoring; could transition into this system step by step. The two systems can even co-exist.<br>
5. It supports an interface where OopClosure is the interface to oop_iterate, rather than ExtendedOopClosure. It uses SFINAE to send metadata info to the closure only if the derived type is an ExtendedOopClosure, otherwise it simply sends the oops (do_oop)
only. (of course I can remove this if it's unwanted and we intentionally don't want to support oop_iterate(OopClosure*) )<br>
<br>
For the interested reader, this is how the old system worked:<br>
The ugly macros generate overloads of oop_iterate on oopDesc which uses a virtual call to the Klass (also using macro generated overloads) to figure out where the oops are and then call the closure. This step with the virtual call to the Klass to call the closure
removes any potential for template magic because template member functions can't be virtual in C++.<br>
<br>
And this is how my system solves this:<br>
A template oop_iterate (with closure type as parameter) member function uses a virtual call to the Klass, but only to acquire information where oops can be found (and NOT to call the actual closure too). It then uses static template polymorphism (CRTP idiom)
to invoke the do_oop method of the corresponding derived closure types (without virtual calls). This required support from the Klass implementations. I currently support object arrays and normal instances. If the Klass implementation does not support this
new scheme, it simply reverts to a normal virtual call like before.<br>
As a bonus I made a new include file in utilities/templateIdioms.hpp with some template magic I needed and which I was missing but could likely be used in more places in the future.<br>
<br>
Would this change be interesting for the GC group? In that case I could prepare a patch (and perhaps add support to the other Klass implementations). :)<br>
I would also need some help to check if this works on your wide range of platforms and compilers etc (only checked the assembly output for my own setup).<br>
</blockquote>
<br>
Yes, please provide a patch!<br>
<br>
I've had a patch to start solving some of these problems for a long time:<br>
<a href="http://cr.openjdk.java.net/~stefank/prototype/oop_iterate_dispatch/webrev.00/">http://cr.openjdk.java.net/~stefank/prototype/oop_iterate_dispatch/webrev.00/</a><br>
<br>
one problem that I haven't solved in my patch is how to pass down the OopClosureType past the Klass::oop_iterate virtual calls. See oop.inline.hpp:<br>
template <bool nv, typename OopClosureType><br>
inline int oopDesc::oop_iterate(OopClosureType* blk) {<br>
CONCRETE_KLASS_DO_AND_RETURN(klass(), oop_oop_iterate<nv>(this,blk));<br>
}<br>
<br>
and klass.inline.hpp for the dispatch code:<br>
<br>
+#define CONCRETE_KLASS_DO_AND_RETURN(the_klass, todo) \<br>
+ do { \<br>
+ switch ((the_klass)->dispatch_tag()) { \<br>
+ case Klass::_instance: return InstanceKlass::cast(the_klass)->todo; \<br>
+ case Klass::_instance_ref: return InstanceRefKlass::cast(the_klass)->todo; \<br>
+ case Klass::_instance_mirror: return InstanceMirrorKlass::cast(the_klass)->todo; \<br>
+ case Klass::_instance_class_loader: return InstanceClassLoaderKlass::cast(the_klass)->todo; \<br>
+ case Klass::_obj_array: return ObjArrayKlass::cast(the_klass)->todo; \<br>
+ case Klass::_type_array: return TypeArrayKlass::cast(the_klass)->todo; \<br>
+ default: fatal("Incorrect dispatch index"); return 0; \<br>
+ } \<br>
+ } while (false)<br>
+<br>
</div>
</blockquote>
<div><br>
</div>
<div>Yeah, I was faced with the same problem. It's impossible in C++ to pass template arguments to virtual member functions. (Note however that it's possible to have template parameterized /types/ with virtual member functions, so with your choice of using
enums with a dispatch tag I think it should be possible to have a KlassDispatchProxy<ClosureType> cl; cl.get_and_dispatch(obj); where KlassDispatchProxy type (rather than Klass type) is picked using the enum and get_and_dispatch is a virtual method for the
parameterized type).</div>
<div><br>
</div>
<div>However, my choice was to instead recognize finding oop maps and dispatching to the closure as two fundamentally separate concerns. So instead I implemented a new virtual Klass member function to only get the oop maps (if supported) of an oop (for InstanceKlass
simply return the array of oop map blocks, for ObjArrayKlass simply return an oop map dummy with the interval of oops for the array). This way, I can get the oop maps with a virtual call without having to know the ClosureType needed for dispatching, and then
when I have the oop maps, simply dispatc (partially using SFINAE to dispatch the stuff the closure expects to receive; depending on if it's an ExtendedOopClosure or not).</div>
<div><br>
</div>
<div>I'm glad this is something useful. Currently I only enable this for InstanceKlass and ObjArrayKlass, otherwise it reverts to standard virtual calls. Will prepare a patch with a few more Klass implementations to make it a bit more complete. :)</div>
<div><br>
</div>
<div>/Erik</div>
<br>
<blockquote type="cite">
<div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">
thanks,<br>
StefanK<br>
<br>
<br>
<blockquote type="cite"><br>
Cheers!<br>
<br>
/Erik</blockquote>
</div>
</blockquote>
</div>
<br>
</body>
</html>