Skip to content

Conversation

david-beaumont
Copy link
Owner

THIS IS NOT FOR CODE REVIEW - DO NOT REVIEW THIS

Rollup of all current preview mode work for Valhalla.
Because the work I've put into JDK mainline (as far back as July) is not yet integrated into Valhalla, this work cannot be considered for submission yet. However, I'm going to put it here for specific people to look at.

Because this is based off a baseline of the changes in mainline (manually patched in), it MUST be view in relation to openjdk#1596 to get the preview mode specific diffs.

jdk_8352750_baseline...david-beaumont:openjdk_valhalla:jdk_8352750_preview_mode

shipilev and others added 30 commits July 1, 2025 07:58
Reviewed-by: thartmann, mhaessig, dfenacci
…offset 8 to null pointer

Reviewed-by: mdoerr, lucy
Reviewed-by: sangheki, kbarrett, tschatzl
Reviewed-by: kbarrett, ayang
…hat no proxy is selected

Reviewed-by: lancea, iris, joehw
Reviewed-by: sviswanathan, sparasa, epeter
…nts::clearall_in_class_at_safepoint

Reviewed-by: coleenp, dholmes, sspitsyn
Reviewed-by: amenkov, dholmes, sspitsyn
Reviewed-by: hchao, mullan, shade
Reviewed-by: aph, tschatzl
… format string

Reviewed-by: kvn, mhaessig, yzheng
Reviewed-by: coleenp, sspitsyn
Reviewed-by: kvn, dfenacci, mdoerr
…e macro elimination

Reviewed-by: kvn, dlunden
coleenp and others added 19 commits September 12, 2025 14:30
…ng convention

Co-authored-by: Tobias Hartmann <[email protected]>
Reviewed-by: thartmann, coleenp
Merge jdk-26+5
…ring.java fails to boot with genzgc

Reviewed-by: coleenp
…ectHeaders and preview mode

Reviewed-by: chagedorn, matsaave
…dingTest.java crashes with segmentation fault

Reviewed-by: coleenp
Copy link
Owner Author

@david-beaumont david-beaumont left a comment

Choose a reason for hiding this comment

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

First round of comments.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Ignore this, it's just a fix I have in my local Valhalla repo for using git workspaces.

Copy link
Owner Author

Choose a reason for hiding this comment

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

This is to get the preview classes into the jimage file by moving the compiled classes into the "META-INF/preview" module subdirectory. It seems to work fine (and has been for months).

Copy link
Owner Author

Choose a reason for hiding this comment

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

Added for testing ImageReader properly.

return SymbolTable::new_symbol(name, pointer_delta_as_int(start, base), pointer_delta_as_int(end, base));
}

// --------------------------------
Copy link
Owner Author

Choose a reason for hiding this comment

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

I added these "micro functions" to just encapsulate everywhere that JImage_file and JImage_mode were used. There's now more complexity than just whether JImage_file is null or not.
Some of these could be considered too small to carry their own weight, but I think it's nice not to see these fields accessed anywhere outside of this code section where they could be used in an invalid way.

}
FreeHeap(valueclasses_dir);
}
// // For each <module>-valueclasses.jar in <JAVA_HOME>/lib/valueclasses/
Copy link
Owner Author

Choose a reason for hiding this comment

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

We MUST NOT submit this as is because it would break the module patching.
We need to figure out what to do here to make a clean transition in this code and the C++ and makefiles.

* @param nameDecoder decoder for module names.
* @return the list of module references for the source package subdirectory.
*/
public static List<ModuleReference> read(
Copy link
Owner Author

Choose a reason for hiding this comment

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

Useful to have this here since it touches the flags (and I wanted to keep those private to avoid confusion between this and the ImageLocation flags). This is called by ImageReader when it "completes" package directories and when it processes preview mode packages.

Looking at it, I think it should be re-worked to not actually return the ref instances since the callers care only about the names.

@@ -0,0 +1,77 @@
/*
Copy link
Owner Author

Choose a reason for hiding this comment

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

TODO: Figure out what to do about ImageReaderFactory.java.

A rename of ImageReaderFactory.java, not sure why it's not showing up as a such here however. I need to make sure that the old class is gone.

@@ -1,5 +1,5 @@
/*
Copy link
Owner Author

Choose a reason for hiding this comment

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

TODO: Figure out what's going on here.

This doesn't handle preview mode yet. I've some code somewhere that should enable it, but I'll have to find it. I think the changes in here are really what's in a mainline PR for exploded image, but I've lost track a bit.

size_t index;
char name_buffer[IMAGE_MAX_PATH];
char* path;
{ // Write the buffer with room to prepend the preview mode infix
Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm using an anonymous scope just to avoid index from needing to be a live variable anywhere else.

index += name_len;
name_buffer[index++] = '\0';
// Path begins at the leading '/' (not the start of the buffer).
path = &name_buffer[preview_infix_len];
Copy link
Owner Author

Choose a reason for hiding this comment

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

It's a bit janky to have my string start some way into the buffer, but it avoid copying the path, which is typically longer than the module name by some margin.

Matias Saavedra Silva and others added 2 commits September 17, 2025 19:58
@david-beaumont david-beaumont force-pushed the jdk_8352750_preview_mode branch from fe0bd75 to ee3dbcf Compare September 18, 2025 23:44
David Simms and others added 6 commits September 19, 2025 08:45
Merge jdk-26+6
Merge jdk-26+7
* Fix test for new ModuleReference behaviour.
* Make preview mode system property dependent in classloader.
* Update benchmark for preview mode and new ImageReader methods.
* Adjusting tests and fixing loose ends from merge.
* Serialization fix
* squash everything latest
@david-beaumont david-beaumont force-pushed the jdk_8352750_preview_mode branch from 58736f7 to 3212420 Compare September 22, 2025 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.