RFR[S] 8005165 Platform-independent C++ vtables for CDS
Ioi Lam
ioi.lam at oracle.com
Wed Mar 8 05:39:43 UTC 2017
On 3/7/17 8:12 PM, Jiangli Zhou wrote:
> Hi Ioi,
>
>> On Mar 7, 2017, at 5:50 PM, Ioi Lam <ioi.lam at oracle.com
>> <mailto:ioi.lam at oracle.com>> wrote:
>>
>>
>>
>> On 3/7/17 4:04 PM, Jiangli Zhou wrote:
>>> Hi Ioi,
>>>
>>> Some minor comments/suggestions for the latest changes:
>>>
>>> You added MetaspaceShared::is_valid_method(). There is an existing
>>> Method::is_valid_method() function. It might cause confusion for
>>> anyone who’s not familiar with the CDS code when trying to determine
>>> which is_valid_method() to use in the future. How about renaming
>>> MetaspaceShared::is_valid_method() to is_valid_archived_method() and
>>> move it to method.*?
>>
>> I renamed it to MetaspaceShared::is_valid_shared_method(), to be
>> consistent with existing names such as
>> MetaspaceShared::is_in_shared_space().
>>
>> I don't want to move it to method.cpp because the logic of
>> determining whether the Method's vptr points to the cloned vtable is
>> inside metaspaceShared.cpp
>
> Ok.
>
>>
>>> Also, MetaspaceShared::is_valid_shared_object() might be more
>>> meaningful if it is rename as
>>> MetaspaceShared::is_valid_shared_metadata or
>>> MetaspaceShared::is_valid_archive_metadata.
>> There's no MetaspaceShared::is_valid_shared_object(). I think you
>> mean CppVtableCloner<T>::is_valid_shared_object.
>> The meaning of the name is pretty clear: is it a valid shared object
>> of type <T>
>
> Using ‘shared_object’ might be a bit too broad here. We have both
> shared metadata and shared String objects. This API is used to
> determine if it is a valid data of the Metadata types that have C++
> vtable. Also, I just noticed the implementation doesn’t do any
> specific ‘is_shared’ check. How about changing it to
> CppVtableCloner<T>::is_valid()?
How about this:
template <class T> class CppVtableCloner : public T {
...
static bool is_valid_shared_object(const T* obj) {
intptr_t* vptr = *(intptr_t**)obj;
return vptr == _info->cloned_vtable();
}
};
bool MetaspaceShared::is_valid_shared_method(const Method* m) {
assert(is_in_shared_space(m), "must be");
return CppVtableCloner<Method>::is_valid_shared_object(m);
}
You can pass in only a Method* to
CppVtableCloner<Method>::is_valid_shared_object(const Method* obj), so
it's pretty clear what "obj" is in this case.
I also added the "is_shared" check as an assert. The caller should call
this only after checking that the method is in shared space.
What do you think?
Thanks
- Ioi
>
> Thanks,
> Jiangli
>
>>
>> Thanks
>> - Ioi
>>>
>>> Thanks,
>>> Jiangli
>>>
>>>> On Mar 7, 2017, at 1:24 PM, Ioi Lam <ioi.lam at oracle.com
>>>> <mailto:ioi.lam at oracle.com>> wrote:
>>>>
>>>> Hi Jiangli,
>>>>
>>>> Good catch!
>>>>
>>>> When I try to remove the code, I actually found my last version
>>>> (v04) would not work with Method::is_valid_method(). The following
>>>> assert would fail. This means that shared methods be skipped when
>>>> we walk the stack (such as when doing a crash dump):
>>>>
>>>> void Method::restore_unshareable_info(TRAPS) {
>>>> - assert(is_method(), "ensure C++ vtable is restored");
>>>> + assert(is_method() && is_valid_method(), "ensure C++ vtable is
>>>> restored");
>>>>
>>>> So I made a more comprehensive fix to make sure
>>>> Method::is_valid_method works. Here's a diff from the previous version:
>>>>
>>>> http://cr.openjdk.java.net/~iklam/jdk10/8005165-platform-independent-cds-vtable.v05/
>>>> <http://cr.openjdk.java.net/%7Eiklam/jdk10/8005165-platform-independent-cds-vtable.v05/>
>>>>
>>>> Thanks
>>>> - Ioi
>>>>
>>>>
>>>> On 3/7/17 9:58 AM, Jiangli Zhou wrote:
>>>>> Hi Ioi,
>>>>>
>>>>> I think there is no other usage of
>>>>> Universe::init_self_patching_vtbl_list() after you removed the
>>>>> ones from metaspaceShared.cpp. You can remove the function from
>>>>> universe.*.
>>>>>
>>>>> Thanks,
>>>>> Jiangli
>>>>>
>>>>>> On Mar 6, 2017, at 5:25 PM, Ioi Lam <ioi.lam at oracle.com> wrote:
>>>>>>
>>>>>> Hi Thomas & Coleen,
>>>>>>
>>>>>> Thanks for your comments. I have updated the rev:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~iklam/jdk10/8005165-platform-independent-cds-vtable.v04/
>>>>>>
>>>>>> [1] Switched back to computing the exact vtable size
>>>>>> [2] Move non-trivial functions outside of their class declaration
>>>>>>
>>>>>> Thanks
>>>>>> - Ioi
>>>>>>
>>>>>> On 3/6/17 8:51 AM, Thomas Stüfe wrote:
>>>>>>> Hi Coleen and Ioi,
>>>>>>>
>>>>>>> I had to port C++ code to platforms with terrible compilers for
>>>>>>> a time in my life, that is why I like code to be as portable as
>>>>>>> possible. That said, you are right in your argumentation, the
>>>>>>> SafeFetch solution is not terribly elegant and Ioi's original
>>>>>>> way of determining the vtable size is cleaner.
>>>>>>>
>>>>>>> I did some checks on some of our architectures with a test
>>>>>>> similar to Ioi's and on a first glance it seems to work for
>>>>>>> simple cases (single and public inheritance) on ppc (AIX) and
>>>>>>> Linux ia64. Although the vtables seemed to me to contain
>>>>>>> function descriptors, not real pointers to code, so this is
>>>>>>> something to keep in mind. But if the live vtable are copied,
>>>>>>> the function descriptors they contain should point to valid code
>>>>>>> too, so it should not matter. Just to remember to not expect
>>>>>>> every slot in the array to be a valid code pointer.
>>>>>>>
>>>>>>> So, in short, I remove my objection to Ioi's original solution,
>>>>>>> as far as that matters.
>>>>>>>
>>>>>>> I still think we rely on a lot here: Contiguous vtable
>>>>>>> containing absolute memory addresses, vtable pointer at start of
>>>>>>> object and vtable entries to be ordered from base->derived
>>>>>>> class. So I wonder how much effort it would be (now or in the
>>>>>>> future as a separate change) to have a fallback where - at
>>>>>>> loading time - instead of copying vtables the vtable pointers in
>>>>>>> the objects were fixed up to point to the new live vtables? I
>>>>>>> know this would be more expensive and potentially defeat the
>>>>>>> point of shared classes. But maybe not, it depends on how many
>>>>>>> objects are there, no?
>>>>>>>
>>>>>>> Kind Regards, Thomas
>>>>>>>
>>>>>>> On Sun, Mar 5, 2017 at 4:17 PM, <coleen.phillimore at oracle.com
>>>>>>> <mailto:coleen.phillimore at oracle.com>> wrote:
>>>>>>>
>>>>>>>
>>>>>>> Ioi, Some comments inline (where no comments, insert "ok") :)
>>>>>>>
>>>>>>>
>>>>>>> On 3/2/17 10:37 PM, Ioi Lam wrote:
>>>>>>>
>>>>>>> Hi Coleen,
>>>>>>>
>>>>>>> Thanks for the comments. I have updated the webrev. See
>>>>>>> in-line for responses.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~iklam/jdk10/8005165-platform-independent-cds-vtable.v03/
>>>>>>> <http://cr.openjdk.java.net/%7Eiklam/jdk10/8005165-platform-independent-cds-vtable.v03/>
>>>>>>>
>>>>>>>
>>>>>>> On 3/2/17 8:48 AM, coleen.phillimore at oracle.com
>>>>>>> <mailto:coleen.phillimore at oracle.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>> Ioi
>>>>>>> I like the concept of this a lot but have some stylistic
>>>>>>> comments to help people reading this code later.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~iklam/jdk10/8005165-platform-independent-cds-vtable.v02/src/share/vm/memory/metaspaceShared.cpp.udiff.html
>>>>>>> <http://cr.openjdk.java.net/%7Eiklam/jdk10/8005165-platform-independent-cds-vtable.v02/src/share/vm/memory/metaspaceShared.cpp.udiff.html>
>>>>>>>
>>>>>>> s/vtab/vtable/g and s/Vtab/Vtable/ please. It doesn't
>>>>>>> save many characters, especially in CppVtableInfo/Testers
>>>>>>>
>>>>>>> Done.
>>>>>>>
>>>>>>> + // Start at slot 1, because slot 0 may be RTTI (on
>>>>>>> Solaris/Sparc)
>>>>>>> + int i;
>>>>>>> + for (i=1; ; i++) {
>>>>>>> Since you're using 'i' later, can you rename it to
>>>>>>> something descriptive. Or have another variable
>>>>>>> "vtable_length" to use later. This looks like an old
>>>>>>> style for loop.
>>>>>>>
>>>>>>> Done
>>>>>>>
>>>>>>> Can the functions for CppVtableInfo be declared
>>>>>>> outside of
>>>>>>> the class declaration? They don't need to be inline and
>>>>>>> then the debug code for testing the vtable size can
>>>>>>> be not
>>>>>>> in the middle of the class declaration. Then you can
>>>>>>> move the Tester classes to inside the same #ifndef
>>>>>>> PRODUCT
>>>>>>> block.
>>>>>>>
>>>>>>> Can you put #endif // PRODUCT when the ifdef covers
>>>>>>> several lines of code?
>>>>>>>
>>>>>>> Done
>>>>>>>
>>>>>>> vtab_of could be more descriptive, like cpp_vtable_for().
>>>>>>>
>>>>>>> I changed to vtable_of(). Because the class name is already
>>>>>>> CppVtableCloner, repeating the word "cpp" seems
>>>>>>> repetitive to me.
>>>>>>>
>>>>>>> Was PrintSharedSpaces was never converted to UL?
>>>>>>>
>>>>>>> Right. I've filed
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8176132
>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8176132>
>>>>>>> (-XX:+PrintSharedSpaces should be converted to use Unified
>>>>>>> Logging.)
>>>>>>>
>>>>>>> + int n = MAX_VTABLE_SIZE;
>>>>>>>
>>>>>>> Can you propagate MAX_VTABLE_SIZE to the places where
>>>>>>> it's
>>>>>>> used. n isn't descriptive. This starts out with
>>>>>>> max_vtable_size and then changes the size. Reusing 'n'
>>>>>>> makes this really hard to follow. Not having a comment
>>>>>>> that we only allocate enough slots for the vtable
>>>>>>> makes it
>>>>>>> hard too.
>>>>>>>
>>>>>>> + // allocate CppVtableInfo in the MD section + _info =
>>>>>>> (CppVtabInfo*)md_top;
>>>>>>> + _info->set_vtab_size(n); // initially set to
>>>>>>> max_vtable_size
>>>>>>> + + // allocate temporary local instance of the metadata
>>>>>>> type T + T tmp;
>>>>>>> + intptr_t* srcvtab = vtab_of(tmp);
>>>>>>> + intptr_t* dstvtab = _info->vtab();
>>>>>>> +
>>>>>>>
>>>>>>> Fixed.
>>>>>>>
>>>>>>> Something like that for comments. dstvtab is the
>>>>>>> destination_vtable in the MD section.
>>>>>>>
>>>>>>>
>>>>>>> I've dropped the md_ prefix from the functions that deal with
>>>>>>> the vtables, since they shouldn't care whether it's the "MD"
>>>>>>> section or not. Now it looks like this:
>>>>>>>
>>>>>>> // Allocate and initialize the C++ vtables, starting from
>>>>>>> top,
>>>>>>> but do not go past end.
>>>>>>> intptr_t*
>>>>>>> MetaspaceShared::allocate_cpp_vtable_clones(intptr_t* top,
>>>>>>> intptr_t* end) {
>>>>>>> assert(DumpSharedSpaces, "dump-time only");
>>>>>>> // Layout (each slot is a intptr_t):
>>>>>>> // [number of slots in the first vtable = n1]
>>>>>>> // [ <n1> slots for the first vtable]
>>>>>>> // [number of slots in the first second = n2]
>>>>>>> // [ <n2> slots for the second vtable]
>>>>>>> // ...
>>>>>>> // The order of the vtables is the same as the
>>>>>>> CPP_VTAB_PATCH_TYPES_DO macro.
>>>>>>> CPP_VTABLE_PATCH_TYPES_DO(ALLOC_CPP_VTABLE_CLONE);
>>>>>>> return top;
>>>>>>> }
>>>>>>>
>>>>>>> + for (int i=0; i<n; i++) {
>>>>>>> + const intptr_t bad = intptr_t(0xdeadbeef);
>>>>>>> + intptr_t num = SafeFetchN(&srcvtab[i], bad);
>>>>>>> + if (num == bad
>>>>>>> + // || i > 120 /* uncomment this line to test */
>>>>>>> + ) {
>>>>>>> + _info->set_vtab_size(i-1);
>>>>>>> + break;
>>>>>>> + }
>>>>>>> + dstvtab[i] = num;
>>>>>>> + }
>>>>>>> I dont understand this code. You get deadbeef for a bad
>>>>>>> value if SafeFetchN gets a fault but why would it get a
>>>>>>> fault at the end of the metadata's vtable? Couldn't it
>>>>>>> just run onto the next vtable? I think your original way
>>>>>>> of counting vtable entries might be better (sorry I
>>>>>>> didn't
>>>>>>> have time to study that thread).
>>>>>>>
>>>>>>> I've modified the comments to this. Does it make sense to
>>>>>>> you?
>>>>>>>
>>>>>>> // It is not always safe to call memcpy(), because
>>>>>>> srcvtable might be shorter than
>>>>>>> // MAX_VTABLE_SIZE, and the C++ linker might have placed
>>>>>>> the vtable at the very
>>>>>>> // end of the last page of libjvm.so. Crossing over
>>>>>>> to the
>>>>>>> next page might
>>>>>>> // cause a page fault.
>>>>>>>
>>>>>>> My fear is the JVM would suddenly start crashing because the
>>>>>>> order of .o files have changed on the linker's command line,
>>>>>>> or if you enable some special linker optimization flags. It's
>>>>>>> better safe than sorry.
>>>>>>>
>>>>>>>
>>>>>>> This wasn't exactly what I was not understanding. I didn't see
>>>>>>> that you are copying 120 entries from the old vtable and junk
>>>>>>> memory beyond the old vtable, unless you get a segv, in which
>>>>>>> case
>>>>>>> you copy less. I don't think you should copy random memory into
>>>>>>> the vtable in the archive. This doesn't seem secure, even with
>>>>>>> the segv protection.
>>>>>>>
>>>>>>> Since we already have assumptions about C++ vtable layout in the
>>>>>>> code and it's mostly specified by various ABIs, and you have the
>>>>>>> assert code, I think I would prefer that you copy only the vtable
>>>>>>> entries into the archive. I guess Thomas Stuefe had a different
>>>>>>> opinion. I've read the original thread. Two points:
>>>>>>>
>>>>>>> If new C++ compiler implementations add a discontigous vtable,
>>>>>>> both the SafeFetchN and subclass additional virtual function at
>>>>>>> end implementation will fail. I don't think C++ implementations
>>>>>>> would do this and a contiguous vtable as first in the
>>>>>>> instance has
>>>>>>> been standard for years. If our metadata adds multiple
>>>>>>> inheritance, the same issue would be a problem for both
>>>>>>> implementations, as well as for the implementation we have before
>>>>>>> Ioi's fix.
>>>>>>>
>>>>>>> Ioi's subclass adding virtual function method would work for any
>>>>>>> esoteric C++ implementations in my memory, except the vptr
>>>>>>> for the
>>>>>>> old DECC++ compiler was after the nonstatic data members (which
>>>>>>> would fail with all of our implementations).
>>>>>>>
>>>>>>> Since the code is there anyway for debug purposes, we're not
>>>>>>> saving code by implementing SafeFetchN. The SafeFetchN
>>>>>>> implementation isn't obvious at all what it's doing, and requires
>>>>>>> better comments, especially if you don't know already what
>>>>>>> SafeFetchN does. It looks really cryptic. The poisoned values
>>>>>>> also bothered me in that they overload other poisoned values in
>>>>>>> other parts of the jvm.
>>>>>>>
>>>>>>> Ioi, could you make all methods of CppVtableCloner out of line?
>>>>>>>
>>>>>>> The other changes look good, although I might have more requests
>>>>>>> for comments.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Coleen
>>>>>>>
>>>>>>>
>>>>>>> Would be nice to have comments here too!!
>>>>>>>
>>>>>>> + intptr_t* start = md_top;
>>>>>>>
>>>>>>> This doesn't do anything (?)
>>>>>>>
>>>>>>>
>>>>>>> Fixed. This was left over code.
>>>>>>>
>>>>>>>
>>>>>>> + MetaspaceShared::zero_cpp_vtable_clones_for_writing();
>>>>>>>
>>>>>>> Why not zero the destination vtable in allocate? Or does
>>>>>>> patching the vtable pointers call virtual functions? You
>>>>>>> could prevent that so you don't need this code.
>>>>>>>
>>>>>>> I added this comment:
>>>>>>>
>>>>>>> // During patching, some virtual methods may be called, so
>>>>>>> at this point
>>>>>>> // the vtables must contain valid methods (as filled in by
>>>>>>> CppVtableCloner::allocate).
>>>>>>> MetaspaceShared::patch_cpp_vtable_pointers();
>>>>>>>
>>>>>>> // The vtable clones contain addresses of the current
>>>>>>> process.
>>>>>>> // We don't want to write these addresses into the archive.
>>>>>>> MetaspaceShared::zero_cpp_vtable_clones_for_writing();
>>>>>>>
>>>>>>> + // Restore the vtable in case we invoke any virtual
>>>>>>> methods.
>>>>>>> +
>>>>>>> MetaspaceShared::clone_cpp_vtables((intptr_t*)vtbl_list);
>>>>>>> Can this be restore_cpp_vtables since that's what it's
>>>>>>> doing. The first is after the dump and the second call is
>>>>>>> at UseSharedSpaces. A couple of comments in this
>>>>>>> clone_cpp_vtables --> restore_cpp_vtables would be
>>>>>>> nice. eg:
>>>>>>>
>>>>>>> I prefer to use the word clone. Otherwise when you just say
>>>>>>> "vtable" it's not clear whether you're talking about the
>>>>>>> original one (made by the c++ linker), or the cloned one in
>>>>>>> the CDS archive.
>>>>>>>
>>>>>>> + static intptr_t* clone_vtable(const char* name,
>>>>>>> intptr_t* p) {
>>>>>>> + T tmp; // Allocate temporary dummy metadata object to
>>>>>>> get vtable initialized
>>>>>>> + CppVtabInfo* info = (CppVtabInfo*)p;
>>>>>>> + int n = info->vtab_size();
>>>>>>> + intptr_t* srcvtab = vtab_of(tmp);
>>>>>>> + intptr_t* dstvtab = info->vtab();
>>>>>>> +
>>>>>>> + // We already checked (and, if necessary, adjusted n)
>>>>>>> when the vtables were allocated, so we are
>>>>>>> + // safe to do memcpy.
>>>>>>> + if (PrintSharedSpaces) {
>>>>>>> + tty->print_cr("%s copying %d vtable entries", name, n);
>>>>>>> + }
>>>>>>> + memcpy(dstvtab, srcvtab, sizeof(intptr_t) * n);
>>>>>>> + return dstvtab + n;
>>>>>>> + }
>>>>>>>
>>>>>>> Done. I changed the wording
>>>>>>>
>>>>>>> T tmp; // Allocate temporary dummy metadata object to get
>>>>>>> to the original vtable.
>>>>>>>
>>>>>>> As we are not really "initializing a vtable" here.
>>>>>>>
>>>>>>> Same with 'patch'. It'd be so much faster and easier to
>>>>>>> read this code with more comments please.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~iklam/jdk10/8005165-platform-independent-cds-vtable.v02/src/share/vm/oops/constantPool.hpp.udiff.html
>>>>>>> <http://cr.openjdk.java.net/%7Eiklam/jdk10/8005165-platform-independent-cds-vtable.v02/src/share/vm/oops/constantPool.hpp.udiff.html>
>>>>>>>
>>>>>>> Why are these testers here?
>>>>>>>
>>>>>>>
>>>>>>> I updated the comment:
>>>>>>>
>>>>>>> // Used by CDS. These classes need to access the private
>>>>>>> ConstantPool() constructor.
>>>>>>> template <class T> friend class CppVtableTesterA;
>>>>>>> template <class T> friend class CppVtableTesterB;
>>>>>>> template <class T> friend class CppVtableCloner;
>>>>>>>
>>>>>>>
>>>>>>> Thanks
>>>>>>> - Ioi
>>>>>>>
>>>>>>>
>>>>>>> On 3/1/17 3:25 AM, Ioi Lam wrote:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8005165
>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8005165>
>>>>>>> http://cr.openjdk.java.net/~iklam/jdk10/8005165-platform-independent-cds-vtable.v02/
>>>>>>> <http://cr.openjdk.java.net/%7Eiklam/jdk10/8005165-platform-independent-cds-vtable.v02/>
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> This is the official review (follow up of the
>>>>>>> "Determining the size of C++ vtables" thread
>>>>>>> onhotspot-dev at openjdk.java.net
>>>>>>> <mailto:onhotspot-dev at openjdk.java.net>).
>>>>>>>
>>>>>>> The new code has the same assumption as the
>>>>>>> existing code in JDK 10: for a C++ object
>>>>>>> that
>>>>>>> contains virtual methods (e.g.,
>>>>>>> ConstantPool),
>>>>>>> we assume the first intptr_t slot of the
>>>>>>> object is a _vptr, which points to a vtable,
>>>>>>> which consists of no more than 150
>>>>>>> intptr_t's.
>>>>>>>
>>>>>>> ConstantPool*p -->[ _vptr ] -------> [
>>>>>>> vtable slot 0 ]
>>>>>>> [ field #0 ] [
>>>>>>> vtable slot 1 ]
>>>>>>> [ field #1 ] [
>>>>>>> vtable slot 2 ]
>>>>>>> [ field #2 ] [
>>>>>>> vtable slot 3 ]
>>>>>>> [ .... ] [
>>>>>>> vtable slot 4]
>>>>>>> [
>>>>>>> vtable slot 5 ]
>>>>>>> [
>>>>>>> ... ]
>>>>>>>
>>>>>>> + In the existing code, we were pointing the
>>>>>>> vtable slots to
>>>>>>> code that's generated by HotSpot.
>>>>>>>
>>>>>>> + In the new code, we copy the vtable slots
>>>>>>> from an existing
>>>>>>> vtable (generated by the C++ linker).
>>>>>>>
>>>>>>> Per Thomas Stüfe's advice, I don't try to
>>>>>>> determine the size of the vtable (as that
>>>>>>> would add one more compiler requirement where
>>>>>>> new virtual methods added by a subclass must
>>>>>>> be placed at a higher offset in the vtable).
>>>>>>>
>>>>>>> Instead, I have added code in non-product
>>>>>>> builds to ensure that the vtables are no
>>>>>>> longer than 150 entries. You can run with
>>>>>>> "-XX:+PrintSharedSpaces -Xshare:dump" to
>>>>>>> print
>>>>>>> out the actual size of the vtables for your
>>>>>>> particular platform:
>>>>>>>
>>>>>>> ConstantPool has 12 virtual methods
>>>>>>> InstanceKlass has 113 virtual methods
>>>>>>> InstanceClassLoaderKlass has 113
>>>>>>> virtual methods
>>>>>>> InstanceMirrorKlass has 113 virtual methods
>>>>>>> InstanceRefKlass has 113 virtual methods
>>>>>>> Method has 12 virtual methods
>>>>>>> ObjArrayKlass has 114 virtual methods
>>>>>>> TypeArrayKlass has 114 virtual methods
>>>>>>>
>>>>>>> As mentioned in the code comments, if you
>>>>>>> have
>>>>>>> an esoteric C++ compiler, the
>>>>>>> verify_sufficient_size() function will
>>>>>>> probably fail, but hopefully that would give
>>>>>>> you some hints for porting this code.
>>>>>>>
>>>>>>> To avoid accidentally touching an unmapped
>>>>>>> page, the code uses SafeFetchN for
>>>>>>> copying the
>>>>>>> vtable contents, and would shrink the vtable
>>>>>>> to less than 150 entries if necessary. I
>>>>>>> can't
>>>>>>> test this for real, but I've added some code
>>>>>>> to simulate an error:
>>>>>>>
>>>>>>> for (int i=0; i<n; i++) {
>>>>>>> const intptr_t bad =
>>>>>>> intptr_t(0xdeadbeef);
>>>>>>> intptr_t num =
>>>>>>> SafeFetchN(&srcvtab[i], bad);
>>>>>>> if (num == bad
>>>>>>> // || i > 120 /* uncomment this
>>>>>>> line
>>>>>>> to test */
>>>>>>> ) {
>>>>>>> _info->set_vtab_size(i-1);
>>>>>>> break;
>>>>>>> }
>>>>>>> dstvtab[i] = num;
>>>>>>> }
>>>>>>>
>>>>>>> Results:
>>>>>>>
>>>>>>> + Removed 850 lines of CPU-dependent code
>>>>>>>
>>>>>>> + CDS image is about 50K smaller
>>>>>>>
>>>>>>> + Previously Metadata objects must live
>>>>>>> in the
>>>>>>> read-write section in the CDS
>>>>>>> archive, because their _vptr was updated at
>>>>>>> run time. Now _vptr is no longer
>>>>>>> updated, so ConstantPool can be moved
>>>>>>> to the
>>>>>>> read-only section (see JDK-8171392).
>>>>>>>
>>>>>>> Thanks
>>>>>>> - Ioi
>
More information about the hotspot-dev
mailing list