Skip to content

Commit cb94dc2

Browse files
christianhaeublchristianhaeubl
committed
[GR-51094] Minor cleanups and simplifications.
PullRequest: graal/16456
2 parents 50c2978 + c824757 commit cb94dc2

File tree

4 files changed

+41
-74
lines changed

4 files changed

+41
-74
lines changed

‎substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/linux/LinuxImageHeapProvider.java

+20-57
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@
4242
import org.graalvm.nativeimage.c.type.CCharPointer;
4343
import org.graalvm.nativeimage.c.type.WordPointer;
4444
import org.graalvm.word.ComparableWord;
45-
import org.graalvm.word.LocationIdentity;
4645
import org.graalvm.word.Pointer;
4746
import org.graalvm.word.PointerBase;
4847
import org.graalvm.word.SignedWord;
@@ -64,8 +63,8 @@
6463
import com.oracle.svm.core.posix.PosixUtils;
6564
import com.oracle.svm.core.posix.headers.Fcntl;
6665
import com.oracle.svm.core.posix.headers.Unistd;
67-
import com.oracle.svm.core.util.PointerUtils;
6866

67+
import jdk.graal.compiler.nodes.NamedLocationIdentity;
6968
import jdk.graal.compiler.word.Word;
7069

7170
/**
@@ -88,10 +87,9 @@ public class LinuxImageHeapProvider extends AbstractImageHeapProvider {
8887

8988
private static final CGlobalData<CCharPointer> PROC_SELF_MAPS = CGlobalDataFactory.createCString("/proc/self/maps");
9089

91-
private static final SignedWord FIRST_ISOLATE_FD = signed(-1);
92-
private static final SignedWord UNASSIGNED_FD = signed(-2);
93-
private static final SignedWord CANNOT_OPEN_FD = signed(-3);
94-
private static final CGlobalData<WordPointer> CACHED_IMAGE_FD = CGlobalDataFactory.createWord(FIRST_ISOLATE_FD);
90+
private static final SignedWord UNASSIGNED_FD = signed(-1);
91+
private static final SignedWord CANNOT_OPEN_FD = signed(-2);
92+
private static final CGlobalData<WordPointer> CACHED_IMAGE_FD = CGlobalDataFactory.createWord(UNASSIGNED_FD);
9593
private static final CGlobalData<WordPointer> CACHED_IMAGE_HEAP_OFFSET = CGlobalDataFactory.createWord();
9694

9795
private static final int MAX_PATHLEN = 4096;
@@ -106,29 +104,15 @@ public boolean guaranteesHeapPreferredAddressSpaceAlignment() {
106104
@Override
107105
@Uninterruptible(reason = "Called during isolate initialization.")
108106
public int initialize(Pointer reservedAddressSpace, UnsignedWord reservedSize, WordPointer basePointer, WordPointer endPointer) {
109-
// If we are the first isolate, we might be able to use the existing image heap (see below)
110-
SignedWord fd = CACHED_IMAGE_FD.get().read();
111-
boolean firstIsolate = false;
112-
if (fd.equal(FIRST_ISOLATE_FD)) {
113-
SignedWord previous = ((Pointer) CACHED_IMAGE_FD.get()).compareAndSwapWord(0, FIRST_ISOLATE_FD, UNASSIGNED_FD, LocationIdentity.ANY_LOCATION);
114-
firstIsolate = previous.equal(FIRST_ISOLATE_FD);
115-
fd = firstIsolate ? UNASSIGNED_FD : previous;
116-
}
117-
118107
/*
119-
* If we haven't already, find and open the image file. Even if we are the first isolate,
120-
* this is necessary because if we fail, we need to leave the loaded image heap in pristine
121-
* condition so we can use it to spawn isolates by copying it.
122-
*
123-
* We cache the file descriptor and the determined offset in the file for subsequent isolate
124-
* initializations. We intentionally allow racing in this step to avoid stalling threads.
108+
* Find and open the image file. We cache the file descriptor and the determined offset in
109+
* the file for subsequent isolate initializations. We intentionally allow racing in this
110+
* step to avoid stalling threads.
125111
*/
126-
if (fd.equal(UNASSIGNED_FD) || firstIsolate) {
112+
SignedWord fd = CACHED_IMAGE_FD.get().read();
113+
if (fd.equal(UNASSIGNED_FD)) {
127114
int opened = openImageFile();
128-
/*
129-
* Pointer cas operations are volatile accesses and prevent code reorderings.
130-
*/
131-
SignedWord previous = ((Pointer) CACHED_IMAGE_FD.get()).compareAndSwapWord(0, fd, signed(opened), LocationIdentity.ANY_LOCATION);
115+
SignedWord previous = ((Pointer) CACHED_IMAGE_FD.get()).compareAndSwapWord(0, fd, signed(opened), NamedLocationIdentity.OFF_HEAP_LOCATION);
132116
if (previous.equal(fd)) {
133117
fd = signed(opened);
134118
} else {
@@ -139,45 +123,24 @@ public int initialize(Pointer reservedAddressSpace, UnsignedWord reservedSize, W
139123
}
140124
}
141125

142-
// If we cannot find or open the image file, fall back to copy it from memory.
126+
/*
127+
* If we cannot find or open the image file, fall back to copy it from memory (the image
128+
* heap must be in pristine condition for that).
129+
*/
143130
if (fd.equal(CANNOT_OPEN_FD)) {
144131
return fallbackCopyingProvider.initialize(reservedAddressSpace, reservedSize, basePointer, endPointer);
145132
}
146133

147134
boolean haveDynamicMethodResolution = DynamicMethodAddressResolutionHeapSupport.isEnabled();
148-
149135
if (haveDynamicMethodResolution) {
150136
int res = DynamicMethodAddressResolutionHeapSupport.get().initialize();
151137
if (res != CEntryPointErrors.NO_ERROR) {
152138
return res;
153139
}
154140
}
155141

156-
// If we are the first isolate and can use the existing image heap, do it.
157142
UnsignedWord pageSize = VirtualMemoryProvider.get().getGranularity();
158-
Word imageHeapBegin = IMAGE_HEAP_BEGIN.get();
159143
UnsignedWord imageHeapSizeInFile = getImageHeapSizeInFile();
160-
int imageHeapOffsetInAddressSpace = Heap.getHeap().getImageHeapOffsetInAddressSpace();
161-
UnsignedWord alignment = WordFactory.unsigned(Heap.getHeap().getPreferredAddressSpaceAlignment());
162-
if (firstIsolate && reservedAddressSpace.isNull() && PointerUtils.isAMultiple(imageHeapBegin, alignment) && imageHeapOffsetInAddressSpace == 0 && !haveDynamicMethodResolution) {
163-
// Mark the whole image heap as read only.
164-
if (VirtualMemoryProvider.get().protect(imageHeapBegin, imageHeapSizeInFile, Access.READ) != 0) {
165-
return CEntryPointErrors.PROTECT_HEAP_FAILED;
166-
}
167-
168-
// Unprotect writable pages.
169-
Pointer writableBegin = IMAGE_HEAP_WRITABLE_BEGIN.get();
170-
UnsignedWord writableSize = IMAGE_HEAP_WRITABLE_END.get().subtract(writableBegin);
171-
if (VirtualMemoryProvider.get().protect(writableBegin, writableSize, Access.READ | Access.WRITE) != 0) {
172-
return CEntryPointErrors.PROTECT_HEAP_FAILED;
173-
}
174-
175-
basePointer.write(imageHeapBegin);
176-
if (endPointer.isNonNull()) {
177-
endPointer.write(IMAGE_HEAP_END.get());
178-
}
179-
return CEntryPointErrors.NO_ERROR;
180-
}
181144

182145
UnsignedWord preHeapRequiredBytes = WordFactory.zero();
183146
if (haveDynamicMethodResolution) {
@@ -191,6 +154,7 @@ public int initialize(Pointer reservedAddressSpace, UnsignedWord reservedSize, W
191154
Pointer heapBase;
192155
Pointer allocatedMemory = WordFactory.nullPointer();
193156
if (reservedAddressSpace.isNull()) {
157+
UnsignedWord alignment = WordFactory.unsigned(Heap.getHeap().getPreferredAddressSpaceAlignment());
194158
heapBase = allocatedMemory = VirtualMemoryProvider.get().reserve(totalAddressSpaceSize, alignment, false);
195159
if (allocatedMemory.isNull()) {
196160
return CEntryPointErrors.RESERVE_ADDRESS_SPACE_FAILED;
@@ -218,22 +182,21 @@ public int initialize(Pointer reservedAddressSpace, UnsignedWord reservedSize, W
218182

219183
// Create memory mappings from the image file.
220184
UnsignedWord fileOffset = CACHED_IMAGE_HEAP_OFFSET.get().read();
221-
Pointer imageHeap = heapBase.add(imageHeapOffsetInAddressSpace);
185+
Pointer imageHeap = getImageHeapBegin(heapBase);
222186
imageHeap = VirtualMemoryProvider.get().mapFile(imageHeap, imageHeapSizeInFile, fd, fileOffset, Access.READ);
223187
if (imageHeap.isNull()) {
224188
freeImageHeap(allocatedMemory);
225189
return CEntryPointErrors.MAP_HEAP_FAILED;
226190
}
227191

228-
Pointer relocPointer = IMAGE_HEAP_A_RELOCATABLE_POINTER.get();
229-
ComparableWord relocatedValue = relocPointer.readWord(0);
230-
ComparableWord mappedValue = imageHeap.readWord(relocPointer.subtract(imageHeapBegin));
192+
Pointer relocsBegin = imageHeap.add(IMAGE_HEAP_RELOCATABLE_BEGIN.get().subtract(IMAGE_HEAP_BEGIN.get()));
193+
ComparableWord relocatedValue = IMAGE_HEAP_A_RELOCATABLE_POINTER.get().readWord(0);
194+
ComparableWord mappedValue = relocsBegin.readWord(0);
231195
if (relocatedValue.notEqual(mappedValue)) {
232196
/*
233197
* Addresses were relocated by dynamic linker, so copy them, but first remap the pages
234198
* to avoid swapping them in from disk.
235199
*/
236-
Pointer relocsBegin = imageHeap.add(IMAGE_HEAP_RELOCATABLE_BEGIN.get().subtract(imageHeapBegin));
237200
UnsignedWord relocsSize = IMAGE_HEAP_RELOCATABLE_END.get().subtract(IMAGE_HEAP_RELOCATABLE_BEGIN.get());
238201
if (!isAMultiple(relocsSize, pageSize)) {
239202
freeImageHeap(allocatedMemory);
@@ -252,7 +215,7 @@ public int initialize(Pointer reservedAddressSpace, UnsignedWord reservedSize, W
252215
}
253216

254217
// Unprotect writable pages.
255-
Pointer writableBegin = imageHeap.add(IMAGE_HEAP_WRITABLE_BEGIN.get().subtract(imageHeapBegin));
218+
Pointer writableBegin = imageHeap.add(IMAGE_HEAP_WRITABLE_BEGIN.get().subtract(IMAGE_HEAP_BEGIN.get()));
256219
UnsignedWord writableSize = IMAGE_HEAP_WRITABLE_END.get().subtract(IMAGE_HEAP_WRITABLE_BEGIN.get());
257220
if (VirtualMemoryProvider.get().protect(writableBegin, writableSize, Access.READ | Access.WRITE) != 0) {
258221
freeImageHeap(allocatedMemory);

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/os/AbstractCopyingImageHeapProvider.java

+5-9
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,6 @@
4242
import com.oracle.svm.core.os.VirtualMemoryProvider.Access;
4343
import com.oracle.svm.core.util.UnsignedUtils;
4444

45-
import jdk.graal.compiler.word.Word;
46-
4745
public abstract class AbstractCopyingImageHeapProvider extends AbstractImageHeapProvider {
4846
@Override
4947
public boolean guaranteesHeapPreferredAddressSpaceAlignment() {
@@ -98,28 +96,26 @@ public int initialize(Pointer reservedAddressSpace, UnsignedWord reservedSize, W
9896
}
9997

10098
// Copy the memory to the reserved address space.
101-
Word imageHeapBegin = IMAGE_HEAP_BEGIN.get();
10299
UnsignedWord imageHeapSizeInFile = getImageHeapSizeInFile();
103-
Pointer imageHeap = heapBase.add(Heap.getHeap().getImageHeapOffsetInAddressSpace());
104-
int result = commitAndCopyMemory(imageHeapBegin, imageHeapSizeInFile, imageHeap);
100+
Pointer imageHeap = getImageHeapBegin(heapBase);
101+
int result = commitAndCopyMemory(IMAGE_HEAP_BEGIN.get(), imageHeapSizeInFile, imageHeap);
105102
if (result != CEntryPointErrors.NO_ERROR) {
106103
freeImageHeap(allocatedMemory);
107104
return result;
108105
}
109106

110107
// Protect the read-only parts at the start of the image heap.
111108
UnsignedWord pageSize = VirtualMemoryProvider.get().getGranularity();
112-
Pointer firstPartOfReadOnlyImageHeap = imageHeap;
113-
UnsignedWord writableBeginPageOffset = UnsignedUtils.roundDown(IMAGE_HEAP_WRITABLE_BEGIN.get().subtract(imageHeapBegin), pageSize);
109+
UnsignedWord writableBeginPageOffset = UnsignedUtils.roundDown(IMAGE_HEAP_WRITABLE_BEGIN.get().subtract(IMAGE_HEAP_BEGIN.get()), pageSize);
114110
if (writableBeginPageOffset.aboveThan(0)) {
115-
if (VirtualMemoryProvider.get().protect(firstPartOfReadOnlyImageHeap, writableBeginPageOffset, Access.READ) != 0) {
111+
if (VirtualMemoryProvider.get().protect(imageHeap, writableBeginPageOffset, Access.READ) != 0) {
116112
freeImageHeap(allocatedMemory);
117113
return CEntryPointErrors.PROTECT_HEAP_FAILED;
118114
}
119115
}
120116

121117
// Protect the read-only parts at the end of the image heap.
122-
UnsignedWord writableEndPageOffset = UnsignedUtils.roundUp(IMAGE_HEAP_WRITABLE_END.get().subtract(imageHeapBegin), pageSize);
118+
UnsignedWord writableEndPageOffset = UnsignedUtils.roundUp(IMAGE_HEAP_WRITABLE_END.get().subtract(IMAGE_HEAP_BEGIN.get()), pageSize);
123119
if (writableEndPageOffset.belowThan(imageHeapSizeInFile)) {
124120
Pointer afterWritableBoundary = imageHeap.add(writableEndPageOffset);
125121
UnsignedWord afterWritableSize = imageHeapSizeInFile.subtract(writableEndPageOffset);

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/os/AbstractImageHeapProvider.java

+8-1
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,14 @@
2727
import static com.oracle.svm.core.Isolates.IMAGE_HEAP_BEGIN;
2828
import static com.oracle.svm.core.Isolates.IMAGE_HEAP_END;
2929

30-
import jdk.graal.compiler.word.Word;
30+
import org.graalvm.word.Pointer;
3131
import org.graalvm.word.UnsignedWord;
3232

3333
import com.oracle.svm.core.Uninterruptible;
3434
import com.oracle.svm.core.heap.Heap;
3535

36+
import jdk.graal.compiler.word.Word;
37+
3638
public abstract class AbstractImageHeapProvider implements ImageHeapProvider {
3739
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
3840
protected static UnsignedWord getImageHeapAddressSpaceSize() {
@@ -45,4 +47,9 @@ protected static UnsignedWord getImageHeapSizeInFile() {
4547
Word imageHeapBegin = IMAGE_HEAP_BEGIN.get();
4648
return IMAGE_HEAP_END.get().subtract(imageHeapBegin);
4749
}
50+
51+
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
52+
protected static Pointer getImageHeapBegin(Pointer heapBase) {
53+
return heapBase.add(Heap.getHeap().getImageHeapOffsetInAddressSpace());
54+
}
4855
}

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/VMThreads.java

+8-7
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,6 @@
2424
*/
2525
package com.oracle.svm.core.thread;
2626

27-
import com.oracle.svm.core.locks.VMLockSupport;
28-
import jdk.graal.compiler.api.directives.GraalDirectives;
29-
import jdk.graal.compiler.api.replacements.Fold;
30-
import jdk.graal.compiler.replacements.ReplacementsUtil;
31-
import jdk.graal.compiler.replacements.nodes.AssertionNode;
3227
import org.graalvm.nativeimage.CurrentIsolate;
3328
import org.graalvm.nativeimage.ImageSingletons;
3429
import org.graalvm.nativeimage.Isolate;
@@ -55,6 +50,7 @@
5550
import com.oracle.svm.core.jdk.UninterruptibleUtils;
5651
import com.oracle.svm.core.jdk.UninterruptibleUtils.AtomicWord;
5752
import com.oracle.svm.core.locks.VMCondition;
53+
import com.oracle.svm.core.locks.VMLockSupport;
5854
import com.oracle.svm.core.locks.VMMutex;
5955
import com.oracle.svm.core.log.Log;
6056
import com.oracle.svm.core.nodes.CFunctionEpilogueNode;
@@ -68,6 +64,11 @@
6864
import com.oracle.svm.core.util.UnsignedUtils;
6965
import com.oracle.svm.core.util.VMError;
7066

67+
import jdk.graal.compiler.api.directives.GraalDirectives;
68+
import jdk.graal.compiler.api.replacements.Fold;
69+
import jdk.graal.compiler.replacements.ReplacementsUtil;
70+
import jdk.graal.compiler.replacements.nodes.AssertionNode;
71+
7172
/**
7273
* Utility methods for the manipulation and iteration of {@link IsolateThread}s.
7374
*/
@@ -407,12 +408,12 @@ public static boolean wasStartedByCurrentIsolate(IsolateThread thread) {
407408
return StartedByCurrentIsolate.getAddress(thread).readByte(0) != 0;
408409
}
409410

410-
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
411+
@Uninterruptible(reason = "Thread locks/holds the THREAD_MUTEX.", callerMustBe = true)
411412
static void lockThreadMutexInNativeCode() {
412413
lockThreadMutexInNativeCode(false);
413414
}
414415

415-
@Uninterruptible(reason = "Called from uninterruptible code.")
416+
@Uninterruptible(reason = "Thread locks/holds the THREAD_MUTEX.", callerMustBe = true)
416417
@NeverInline("Must not be inlined in a caller that has an exception handler: We only support InvokeNode and not InvokeWithExceptionNode between a CFunctionPrologueNode and CFunctionEpilogueNode.")
417418
private static void lockThreadMutexInNativeCode(boolean unspecifiedOwner) {
418419
CFunctionPrologueNode.cFunctionPrologue(StatusSupport.STATUS_IN_NATIVE);

0 commit comments

Comments
 (0)