Automatic Closure Specialization
Stefan Karlsson
stefan.karlsson at oracle.com
Wed Oct 8 13:43:30 UTC 2014
On 2014-10-08 15:11, Erik Österlund wrote:
> Hi Stefan,
>
> On 08 Oct 2014, at 14:30, Stefan Karlsson <stefan.karlsson at oracle.com
> <mailto:stefan.karlsson at oracle.com>> wrote:
>
>> Hi Erik,
>>
>> On 2014-10-08 13:20, Erik Österlund wrote:
>>> Greetings,
>>>
>>> So here in this email is attached a patch-file containing my changes
>>> since I don't have access to the cr-server.
>>> If anyone could help me make a RFR with a bug-ID with this
>>> changeset, that would be great.
>>
>> Webrev: http://cr.openjdk.java.net/~stefank/eriko/8059936/webrev.00/
>> <http://cr.openjdk.java.net/%7Estefank/eriko/8059936/webrev.00/>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8059936
>> <https://bugs.openjdk.java.net/browse/JDK-8059936>
>>
>> I ran this through JPRT and got this failure on Solaris x86:
>> "hotspot/src/share/vm/memory/specialized_oop_closures.hpp", line 222: Warning: Identifier expected instead of "}".
>> 1 Warning(s) detected.
>> gmake[8]: *** [abstractCompiler.o] Error 2
>> gmake[7]: *** [the_vm] Error 2
>> gmake[6]: *** [fastdebug] Error 2
>> gmake[5]: *** [generic_build2] Error 2
>> gmake[4]: *** [fastdebug] Error 2
> Here's a new take where I declare my enum on that line with a compiler
> helmet on. The new archive contains the _incremental and _full patch
> to fix this issue.
http://cr.openjdk.java.net/~stefank/eriko/8059936/webrev.01.delta/
http://cr.openjdk.java.net/~stefank/eriko/8059936/webrev.01/
> There's no way I can run JPRT myself is there?
Unfortunately, no.
This patch fails on some platforms that don't use precompiled headers.
Can you compile with USE_PRECOMPILED_HEADER=0 and take care of the
compile errors?
I saw that one of those failures happened after
specialized_oop_closures.inline.hpp was included from oop.hpp:
http://cr.openjdk.java.net/~stefank/eriko/8059936/webrev.01/src/share/vm/oops/oop.hpp.udiff.html
You need to rework you dependencies so that you don't include
.inline.hpp files from .hpp files.
See the new Files section of:
https://wiki.openjdk.java.net/display/HotSpot/StyleGuide
cheers,
StefanK
>
> Thank you,
>
> /Erik
>
>
>> StefanK
>>
>>>
>>> == Implementation Summary ==
>>>
>>> === Dispatching To Closures ===
>>> All dispatching to OopClosure go through an all static class
>>> OopClosureDispatcher. It uses SFINAE to find the most appropriate
>>> member function to call by checking certain conditions of the
>>> OopClosureType. For any template parameterized OopClosureType calls
>>> to member function X in {do_oop, do_metadata, do_klass,
>>> do_class_loader_data} it will attempt the following in this order:
>>> 1) Check if the OopClosureType is manually unspecialized for
>>> whatever reason (currently only used by the abstract classes
>>> OopClosure and ExtendedOopClosure), in that case use virtual call.
>>> 2) Check if OopClosureType (and not a parent) declares X_nv, then
>>> use a non-virtual call to it. Otherwise 3) check if OopClosureType
>>> (and not a parent) declares X, then use a non-virtual call to X
>>> (hence removing need to define both X and X_nv any more, but still
>>> being backward compatible). Otherwise 3) use a normal virtual call to X.
>>>
>>> The reason why checking that candidates for non-virtual calls are
>>> declared in the concrete OopClosureType and not a parent is to be
>>> safe rather than sorry in detecting "abstract" base classes that are
>>> never used by themselves. However their derived types are used, but
>>> the type passed in to oop_iterate is not the derived closure type
>>> but the abstract one. So in summary we make sure we make non-virtual
>>> calls to the derived closure types and not the base types.
>>>
>>> === Dispatching To Klass ===
>>> Two mechanisms are used for retaining the OopClosureType when
>>> dispatching to it using a specific Klass.
>>> Attempt one hopes there is a separation of concern between finding
>>> oops and dispatching to OopClosureType. A virtual call is made to
>>> the Klass to either a) return back the oop maps or b) identify which
>>> Klass it is with a "DispatchTag".
>>> If oop maps are returned (InstanceKlass, OopArrayKlass,
>>> TypeArrayKlass) then they are iterated over from oop_iterate where
>>> all the type info about the closure is still known. If this is not
>>> supported (InstanceRefKlass, InstanceMirrorKlass,
>>> InstanceClassLoaderKlass), then the dispatch tag is used to call an
>>> inline template method defined in brand new
>>> Instance*Klass.inline.hpp files.
>>>
>>> The new methods in Instance*Klass.inline.hpp are (non-virtual)
>>> template variants of what the old oop_oop_iterate macros did. They
>>> use SFINAE to check if the OopClosureType expects metadata related
>>> calls too (if it's an ExtendedOopClosure) otherwise does nothing.
>>> Note that oop_iterate now takes an arbitrary type instead of
>>> ExtendedOopClosure, but it uses SFINAE to force template
>>> instantiations only if it's a subtype of ExtendedOopClosure and
>>> otherwise generate compiler errors and similarly on
>>> oop_iterate_no_header, specialization is used and it explicitly
>>> checks the template parameter is of type OopClosure. If it's an
>>> ExtendedOopClosure, it sends the first ever composite closure type
>>> NoHeaderOopClosure<OopClosureType> to the dispatch system so that
>>> metadata-related calls are never called in the first place. :)
>>>
>>> Thanks,
>>>
>>> /Erik
>>>
>>> On 01 Oct 2014, at 17:02, Erik Österlund <erik.osterlund at lnu.se
>>> <mailto:erik.osterlund at lnu.se>> wrote:
>>>
>>>>
>>>> On 01 Oct 2014, at 16:11, Stefan Karlsson
>>>> <stefan.karlsson at oracle.com <mailto:stefan.karlsson at oracle.com>> wrote:
>>>>
>>>>>
>>>>> On 2014-10-01 15:41, Erik Österlund wrote:
>>>>>> Hey,
>>>>>>
>>>>>> I have to admit I'm not a huge fan of the current system for
>>>>>> explicitly specializing closures (removing virtual calls).
>>>>>>
>>>>>> Here's a couple of problems I see with the current solution:
>>>>>> 1. The specialization macros are obviously not pretty
>>>>>> 2. It's awkward to have to remember to explicitly list the
>>>>>> closure as specialized in the macros
>>>>>> 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.
>>>>>> 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.
>>>>>>
>>>>>> With my alternative template magic solution:
>>>>>> 1. You won't have to explicitly specialize wanted closure types -
>>>>>> they are automatically specialized unless the contrary is
>>>>>> explicitly stated.
>>>>>> 2. Parameterized composite closure types can be used without
>>>>>> unnecessary virtual call overheads.
>>>>>> 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.
>>>>>> 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.
>>>>>> 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*) )
>>>>>>
>>>>>> For the interested reader, this is how the old system worked:
>>>>>> 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++.
>>>>>>
>>>>>> And this is how my system solves this:
>>>>>> 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.
>>>>>> 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.
>>>>>>
>>>>>> 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). :)
>>>>>> 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).
>>>>>
>>>>> Yes, please provide a patch!
>>>>>
>>>>> I've had a patch to start solving some of these problems for a
>>>>> long time:
>>>>> http://cr.openjdk.java.net/~stefank/prototype/oop_iterate_dispatch/webrev.00/
>>>>> <http://cr.openjdk.java.net/%7Estefank/prototype/oop_iterate_dispatch/webrev.00/>
>>>>>
>>>>> 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:
>>>>> template <bool nv, typename OopClosureType>
>>>>> inline int oopDesc::oop_iterate(OopClosureType* blk) {
>>>>> CONCRETE_KLASS_DO_AND_RETURN(klass(), oop_oop_iterate<nv>(this,blk));
>>>>> }
>>>>>
>>>>> and klass.inline.hpp for the dispatch code:
>>>>>
>>>>> +#define CONCRETE_KLASS_DO_AND_RETURN(the_klass, todo) \
>>>>> + do { \
>>>>> + switch ((the_klass)->dispatch_tag()) { \
>>>>> + case Klass::_instance: return
>>>>> InstanceKlass::cast(the_klass)->todo; \
>>>>> + case Klass::_instance_ref: return
>>>>> InstanceRefKlass::cast(the_klass)->todo; \
>>>>> + case Klass::_instance_mirror: return
>>>>> InstanceMirrorKlass::cast(the_klass)->todo; \
>>>>> + case Klass::_instance_class_loader: return
>>>>> InstanceClassLoaderKlass::cast(the_klass)->todo; \
>>>>> + case Klass::_obj_array: return
>>>>> ObjArrayKlass::cast(the_klass)->todo; \
>>>>> + case Klass::_type_array: return
>>>>> TypeArrayKlass::cast(the_klass)->todo; \
>>>>> + default: fatal("Incorrect dispatch index"); return 0; \
>>>>> + } \
>>>>> + } while (false)
>>>>> +
>>>>
>>>> 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).
>>>>
>>>> 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).
>>>>
>>>> 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. :)
>>>>
>>>> /Erik
>>>>
>>>>> thanks,
>>>>> StefanK
>>>>>
>>>>>
>>>>>>
>>>>>> Cheers!
>>>>>>
>>>>>> /Erik
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20141008/7c9c1604/attachment.htm>
More information about the hotspot-gc-dev
mailing list