Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor/fix: Use object offsets instead of field indexes for precise object scanning #3736

Merged
merged 3 commits into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 8 additions & 14 deletions nativelib/src/main/resources/scala-native/gc/commix/Marker.c
Original file line number Diff line number Diff line change
Expand Up @@ -208,22 +208,16 @@ int Marker_markRegularObject(Heap *heap, Stats *stats, Object *object,
GreyPacket **outHolder,
GreyPacket **outWeakRefHolder, Bytemap *bytemap) {
int objectsTraced = 0;
int64_t *ptr_map = object->rtti->refMapStruct;

for (int i = 0; ptr_map[i] != LAST_FIELD_OFFSET; i++) {
word_t current = ptr_map[i];
if (Object_IsReferantOfWeakReference(object, ptr_map[i]))
int32_t *refFieldOffsets = object->rtti->refFieldOffsets;
for (int i = 0; refFieldOffsets[i] != LAST_FIELD_OFFSET; i++) {
size_t fieldOffset = (size_t)refFieldOffsets[i];
Field_t *fieldRef = (Field_t *)((int8_t *)object + fieldOffset);
Field_t fieldReferant = *fieldRef;
if (Object_IsReferantOfWeakReference(object, fieldOffset)) {
continue;

word_t *field = object->fields[current];
if (Heap_IsWordInHeap(heap, field)) {
ObjectMeta *fieldMeta = Bytemap_Get(bytemap, field);
if (ObjectMeta_IsAllocated(fieldMeta)) {
Marker_markObject(heap, stats, outHolder, outWeakRefHolder,
bytemap, (Object *)field, fieldMeta);
}
objectsTraced += 1;
}
objectsTraced += Marker_markField(heap, stats, outHolder,
outWeakRefHolder, fieldReferant);
}
if (object->rtti->rt.id == __boxed_ptr_id) {
// Boxed ptr always has a single field
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,16 @@ static void WeakRefGreyList_NullifyPacket(Heap *heap, Stats *stats,
Object *object = GreyPacket_Pop(weakRefsPacket);
assert(Object_IsWeakReference(object));

word_t fieldOffset = __weak_ref_field_offset;
word_t *refObject = object->fields[fieldOffset];
if (Heap_IsWordInHeap(heap, refObject)) {
ObjectMeta *objectMeta = Bytemap_Get(bytemap, refObject);
if (ObjectMeta_IsAllocated(objectMeta)) {
if (!ObjectMeta_IsMarked(objectMeta)) {
object->fields[fieldOffset] = NULL;
// idempotent operation - does not need to be synchronized
anyVisited = true;
}
Object **weakRefReferantField =
(Object **)((int8_t *)object + __weak_ref_field_offset);
word_t *weakRefReferant = (word_t *)*weakRefReferantField;
if (Heap_IsWordInHeap(heap, weakRefReferant)) {
ObjectMeta *objectMeta = Bytemap_Get(bytemap, weakRefReferant);
if (ObjectMeta_IsAllocated(objectMeta) &&
!ObjectMeta_IsMarked(objectMeta)) {
*weakRefReferantField = NULL;
// idempotent operation - does not need to be synchronized
anyVisited = true;
}
}
}
Expand Down
10 changes: 6 additions & 4 deletions nativelib/src/main/resources/scala-native/gc/immix/Marker.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,13 @@ void Marker_Mark(Heap *heap, Stack *stack) {
}
// non-object arrays do not contain pointers
} else {
int64_t *ptr_map = object->rtti->refMapStruct;
for (int i = 0; ptr_map[i] != LAST_FIELD_OFFSET; i++) {
if (Object_IsReferantOfWeakReference(object, ptr_map[i]))
int32_t *refFieldOffsets = object->rtti->refFieldOffsets;
for (int i = 0; refFieldOffsets[i] != LAST_FIELD_OFFSET; i++) {
size_t fieldOffset = (size_t)refFieldOffsets[i];
Field_t *fieldRef = (Field_t *)((int8_t *)object + fieldOffset);
if (Object_IsReferantOfWeakReference(object, fieldOffset))
continue;
Marker_markField(heap, stack, object->fields[ptr_map[i]]);
Marker_markField(heap, stack, *fieldRef);
}
if (objectId == __boxed_ptr_id) {
// Boxed ptr always has a single field
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,16 @@ void WeakRefStack_Nullify(void) {
Bytemap *bytemap = heap.bytemap;
while (!Stack_IsEmpty(&weakRefStack)) {
Object *object = Stack_Pop(&weakRefStack);
int64_t fieldOffset = __weak_ref_field_offset;
word_t *refObject = object->fields[fieldOffset];
if (Heap_IsWordInHeap(&heap, refObject)) {
ObjectMeta *objectMeta = Bytemap_Get(bytemap, refObject);
if (!ObjectMeta_IsMarked(objectMeta)) {
Object **weakRefReferantField =
(Object **)((int8_t *)object + __weak_ref_field_offset);
word_t *weakRefReferant = (word_t *)*weakRefReferantField;
if (Heap_IsWordInHeap(&heap, weakRefReferant)) {
ObjectMeta *objectMeta = Bytemap_Get(bytemap, weakRefReferant);
if (ObjectMeta_IsAllocated(objectMeta) &&
!ObjectMeta_IsMarked(objectMeta)) {
// WeakReferences should have the held referent
// field set to null if collected
object->fields[fieldOffset] = NULL;
*weakRefReferantField = NULL;
visited = true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#include <stdbool.h>
#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>
#include <wchar.h>
#include "immix_commix/CommonConstants.h"
#include "immix_commix/Log.h"
#include "immix_commix/utils/MathUtils.h"
Expand All @@ -29,6 +31,8 @@ extern const int __boxed_ptr_id;

#endif

typedef struct StringObject StringObject;

typedef struct {
struct {
word_t *cls;
Expand All @@ -37,11 +41,12 @@ typedef struct {
#endif
int32_t id;
int32_t tid;
word_t *name;
StringObject *name;
} rt;
int32_t size;
int32_t idRangeUntil;
int64_t *refMapStruct;
int32_t *refFieldOffsets; // Array of field offsets (in bytes) from object
// start, terminated with -1
} Rtti;

typedef word_t *Field_t;
Expand All @@ -63,6 +68,25 @@ typedef struct {
int32_t stride;
} ArrayHeader;

typedef struct {
ArrayHeader header;
uint16_t values[0];
} CharArray;

typedef struct StringObject {
// ObjectHeader
Rtti *rtti;
#ifdef USES_LOCKWORD
word_t *lockWord;
#endif
// Object fields
// Best effort, order of fields is not guaranteed
CharArray *value;
int32_t offset;
int32_t count;
int32_t cached_hash_code;
} StringObject;

typedef struct Chunk Chunk;

struct Chunk {
Expand Down Expand Up @@ -123,4 +147,22 @@ static inline Field_t Field_allignedLockRef(const Field_t field) {
}
#endif

/* Returns a wide string containg Class.name of given object based on UTF-8
* java.lang.String value.
* Caller of this function is responsible for freeing returned pointer. Function
* can fail if StringObject layout does not match the runtime layout
*/
static inline wchar_t *Object_nameWString(Object *object) {
// Depending on platform wchar_t might be 2 or 4 bytes
// Always convert Scala Char to wchar_t
CharArray *strChars = object->rtti->rt.name->value;
int nameLength = strChars->header.length;
wchar_t *buf = calloc(nameLength + 1, sizeof(wchar_t));
for (int i = 0; i < nameLength; i++) {
buf[i] = (wchar_t)strChars->values[i];
}
buf[nameLength] = 0;
return buf;
}

#endif // IMMIX_OBJECTHEADER_H
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class CommonMemoryLayouts(implicit meta: Metadata) {
Rtti.layout ::
nir.Type.Int :: // class size
nir.Type.Int :: // id range
FieldLayout.referenceOffsetsTy :: // reference offsets
nir.Type.Ptr :: // reference offsets
dynMapType.toList

override val layout =
Expand Down
14 changes: 2 additions & 12 deletions tools/src/main/scala/scala/scalanative/codegen/FieldLayout.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,6 @@ package codegen

import scalanative.linker.{Class, Field}

object FieldLayout {

val referenceOffsetsTy = nir.Type.StructValue(Seq(nir.Type.Ptr))

}

class FieldLayout(cls: Class)(implicit meta: Metadata) {

import meta.layouts.{Object, ObjectHeader}
Expand Down Expand Up @@ -39,12 +33,8 @@ class FieldLayout(cls: Class)(implicit meta: Metadata) {

val struct = nir.Type.StructValue(layout.tys.map(_.ty))
val size = layout.size
val referenceOffsetsValue = nir.Val.StructValue(
Seq(
nir.Val.Const(
nir.Val.ArrayValue(nir.Type.Long, layout.referenceFieldsOffsets)
)
)
val referenceOffsetsValue = nir.Val.Const(
nir.Val.ArrayValue(nir.Type.Int, layout.referenceFieldsOffsets)
)

}
57 changes: 36 additions & 21 deletions tools/src/main/scala/scala/scalanative/codegen/Generate.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package codegen
import scala.collection.mutable
import scala.scalanative.linker.{
Class,
Field,
ScopeInfo,
Unavailable,
ReachabilityAnalysis
Expand Down Expand Up @@ -523,28 +524,42 @@ object Generate {
nir.Val.Int(value)
)

val (weakRefIdsMin, weakRefIdsMax, modifiedFieldOffset) = reachabilityAnalysis.infos
.get(nir.Global.Top("java.lang.ref.WeakReference"))
val WeakReferenceClass = nir.Global.Top("java.lang.ref.WeakReference")
val WeakReferenceGCReferent = WeakReferenceClass.member(
nir.Sig.Field("_gc_modified_referent")
)
def weakRefClsInfo = reachabilityAnalysis.infos
.get(WeakReferenceClass)
.collect { case cls: Class if cls.allocated => cls }
.fold((-1, -1, -1)) { weakRef =>
// if WeakReferences are being compiled and therefore supported
val gcModifiedFieldIndexes: Seq[Int] =
meta.layout(weakRef).entries.zipWithIndex.collect {
case (field, index) if field.name.mangle.contains("_gc_modified_") =>
index
}

if (gcModifiedFieldIndexes.size != 1)
throw new Exception(
"Exactly one field should have the \"_gc_modified_\" modifier in java.lang.ref.WeakReference"
)

(
meta.ranges(weakRef).start,
meta.ranges(weakRef).end,
gcModifiedFieldIndexes.head
)
}
def weakRefReferentField =
reachabilityAnalysis.infos
.get(WeakReferenceGCReferent)
.collect { case field: Field => field }

val (weakRefIdsMin, weakRefIdsMax, modifiedFieldOffset) =
weakRefClsInfo
.zip(weakRefReferentField)
.headOption
.fold((-1, -1, -1)) {
case (weakRef, weakRefReferantField) =>
// if WeakReferences are being compiled and therefore supported
val layout = meta.layout(weakRef)
val gcModifiedFieldReferentIdx = layout
.index(weakRefReferantField)
.ensuring(
_ > 0,
"Runtime implementaiton error, no \"_gc_modified_referent\" field in java.lang.ref.WeakReference"
WojciechMazur marked this conversation as resolved.
Show resolved Hide resolved
)
val gcModifiedFieldReferentOffset = layout.layout
.tys(gcModifiedFieldReferentIdx)
.offset

(
meta.ranges(weakRef).start,
meta.ranges(weakRef).end,
gcModifiedFieldReferentOffset.toInt
)
}
addToBuf(weakRefIdsMaxName, weakRefIdsMax)
addToBuf(weakRefIdsMinName, weakRefIdsMin)
addToBuf(weakRefFieldOffsetName, modifiedFieldOffset)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,22 @@ final case class MemoryLayout(
* end expresed as number of words. Used by the GC for scanning objects.
* Terminated with offset=-1
*/
def referenceFieldsOffsets(implicit meta: Metadata): Seq[nir.Val.Long] = {
val sizeOfHeader = meta.layouts.ObjectHeader.size
private[codegen] def referenceFieldsOffsets(implicit
meta: Metadata
): Seq[nir.Val.Int] = {
import nir.Type._
val ptrOffsets =
val offsets =
tys.collect {
// offset in words without object header
case MemoryLayout.PositionedType(
_: RefKind | // normal or alligned reference field
StructValue((_: RefKind) :: ArrayValue(nir.Type.Byte, _) :: Nil),
offset
) =>
nir.Val.Long(
(offset - sizeOfHeader) / MemoryLayout.BYTES_IN_LONG
)
Comment on lines -35 to -37
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way on calculating reference field indexes could have lead to runtime issues in 32-bit runtime. It would lead to having double indexes (eg. offset=16 & offset=20 would point to the same index) and might omit scanning some of misaligned fields.

offset.toInt
}

ptrOffsets :+ nir.Val.Long(-1)
(offsets :+ -1).map(nir.Val.Int(_))
}

/** A list of offsets pointing to all inner reference types excluding object
Expand Down