RFR: 4718400: Many quantities are held as signed that should be unsigned

Coleen Phillimore coleenp at openjdk.java.net
Mon Oct 25 12:08:04 UTC 2021


On Mon, 25 Oct 2021 08:42:20 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

> The GC code tends to use the size_t type to describe sizes of memory and objects, while the oopDesc class and *Klasses use int. The unit of the object size is in "words" (8 bytes on 64-bits, 4 bytes on 32-bits), and int variables can currently hold the largest object size in words. However, when/if we start to experiment with larger objects, using int could be limiting. I propose that we convert oopDesc::size to return size_t to describe the "objects size"-in-words quantities, and update calling code to also use size_t. Note that there will still be places in the JVM that expects the size to fit in 4 bytes. I've listed a couple of them below.
> 
> I'm reusing this old bug, which more or less covers what this patch does. It would probably be good to update the name of the bug if we decide this patch should be integrated, but I given the age of the RFE, I decided to open the PR with its current name. :)
> 
> Comments on the patch:
> * There are some JVMCI usages of 'long' (not jlong). It would be good to figure out why that is, and if that's something that should be cleaned out.
> * JFR stores field offsets in bytes in 4 bytes variables. This could be a bug. I added an assert.
> *  java_lang_Class::set_oop_size/oop_size uses size_t but the stored field is only 4 bytes. Leaving this for future consideration.
> * I left array object size calculation as-is for now.
> 
> I've run this through tier1-3. I'll run more tiers after initial feedback on this proposal

The runtime changes look great!

src/hotspot/share/classfile/javaClasses.cpp line 1383:

> 1381:   assert(_oop_size_offset != 0, "must be set");
> 1382:   assert(size > 0, "Oop size must be greater than zero, not " SIZE_FORMAT, size);
> 1383:   assert(size <= INT_MAX, "Lossy conversion: " SIZE_FORMAT, size);

Yes, can you add a small comment here that the size in the java.lang.Class instance is only 4 bytes?  It probably doesn't need to be but not something to change in this change.

-------------

Marked as reviewed by coleenp (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/6100


More information about the hotspot-dev mailing list