-
Notifications
You must be signed in to change notification settings - Fork 124
8368467: [lworld] Add new flag generation for jimage to support preview mode #1621
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
base: pr/1619
Are you sure you want to change the base?
Changes from all commits
936b22d
8858134
d009eaa
b192c84
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,7 +53,7 @@ boolean resolve() { | |
ENABLED() { | ||
@Override | ||
boolean resolve() { | ||
return true; | ||
return ENABLE_PREVIEW_MODE; | ||
} | ||
}, | ||
/** | ||
|
@@ -63,6 +63,9 @@ boolean resolve() { | |
FOR_RUNTIME() { | ||
@Override | ||
boolean resolve() { | ||
if (!ENABLE_PREVIEW_MODE) { | ||
return false; | ||
} | ||
Comment on lines
+66
to
+68
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As commented in the 1618 PR, the extra subclasses caused by the overrides can be replaced by the resolve method switching on |
||
// We want to call jdk.internal.misc.PreviewFeatures.isEnabled(), but | ||
// is not available in older JREs, so we must look to it reflectively. | ||
Class<?> clazz; | ||
|
@@ -81,6 +84,15 @@ boolean resolve() { | |
} | ||
}; | ||
|
||
// Temporary system property to disable preview patching and enable the new preview mode | ||
// feature for testing/development. Once the preview mode feature is finished, the value | ||
// will be always 'true' and this code, and all related dead-code can be removed. | ||
private static final boolean DISABLE_PREVIEW_PATCHING_DEFAULT = false; | ||
private static final boolean ENABLE_PREVIEW_MODE = Boolean.parseBoolean( | ||
System.getProperty( | ||
"DISABLE_PREVIEW_PATCHING", | ||
Boolean.toString(DISABLE_PREVIEW_PATCHING_DEFAULT))); | ||
|
||
/** | ||
* Resolves whether preview mode should be enabled for an {@link ImageReader}. | ||
*/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
/* | ||
* Copyright (c) 2014, 2024, Oracle and/or its affiliates. All rights reserved. | ||
* Copyright (c) 2014, 2025, Oracle and/or its affiliates. All rights reserved. | ||
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. | ||
* | ||
* This code is free software; you can redistribute it and/or modify it | ||
|
@@ -49,6 +49,7 @@ | |
import java.util.stream.Collectors; | ||
import java.util.stream.Stream; | ||
|
||
import jdk.internal.jimage.ImageLocation; | ||
import jdk.tools.jlink.internal.Archive.Entry; | ||
import jdk.tools.jlink.internal.Archive.Entry.EntryType; | ||
import jdk.tools.jlink.internal.JRTArchive.ResourceFileEntry; | ||
|
@@ -227,31 +228,8 @@ private static ResourcePool generateJImage(ResourcePoolManager allContent, | |
DataOutputStream out, | ||
boolean generateRuntimeImage | ||
) throws IOException { | ||
ResourcePool resultResources; | ||
try { | ||
resultResources = pluginSupport.visitResources(allContent); | ||
if (generateRuntimeImage) { | ||
// Keep track of non-modules resources for linking from a run-time image | ||
resultResources = addNonClassResourcesTrackFiles(resultResources, | ||
writer); | ||
// Generate the diff between the input resources from packaged | ||
// modules in 'allContent' to the plugin- or otherwise | ||
// generated-content in 'resultResources' | ||
resultResources = addResourceDiffFiles(allContent.resourcePool(), | ||
resultResources, | ||
writer); | ||
} | ||
} catch (PluginException pe) { | ||
if (JlinkTask.DEBUG) { | ||
pe.printStackTrace(); | ||
} | ||
throw pe; | ||
} catch (Exception ex) { | ||
if (JlinkTask.DEBUG) { | ||
ex.printStackTrace(); | ||
} | ||
throw new IOException(ex); | ||
} | ||
ResourcePool resultResources = | ||
getResourcePool(allContent, writer, pluginSupport, generateRuntimeImage); | ||
Set<String> duplicates = new HashSet<>(); | ||
long[] offset = new long[1]; | ||
|
||
|
@@ -282,8 +260,10 @@ private static ResourcePool generateJImage(ResourcePoolManager allContent, | |
offset[0] += onFileSize; | ||
return; | ||
} | ||
int locFlags = ImageLocation.getFlags( | ||
res.path(), p -> resultResources.findEntry(p).isPresent()); | ||
duplicates.add(path); | ||
writer.addLocation(path, offset[0], compressedSize, uncompressedSize); | ||
writer.addLocation(path, offset[0], compressedSize, uncompressedSize, locFlags); | ||
paths.add(path); | ||
offset[0] += onFileSize; | ||
} | ||
|
@@ -307,6 +287,40 @@ private static ResourcePool generateJImage(ResourcePoolManager allContent, | |
return resultResources; | ||
} | ||
|
||
private static ResourcePool getResourcePool( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pulled out so the variable holding it is effectively final and can be used in a lambda expression. |
||
ResourcePoolManager allContent, | ||
BasicImageWriter writer, | ||
ImagePluginStack pluginSupport, | ||
boolean generateRuntimeImage) | ||
throws IOException { | ||
ResourcePool resultResources; | ||
try { | ||
resultResources = pluginSupport.visitResources(allContent); | ||
if (generateRuntimeImage) { | ||
// Keep track of non-modules resources for linking from a run-time image | ||
resultResources = addNonClassResourcesTrackFiles(resultResources, | ||
writer); | ||
// Generate the diff between the input resources from packaged | ||
// modules in 'allContent' to the plugin- or otherwise | ||
// generated-content in 'resultResources' | ||
resultResources = addResourceDiffFiles(allContent.resourcePool(), | ||
resultResources, | ||
writer); | ||
} | ||
} catch (PluginException pe) { | ||
if (JlinkTask.DEBUG) { | ||
pe.printStackTrace(); | ||
} | ||
throw pe; | ||
} catch (Exception ex) { | ||
if (JlinkTask.DEBUG) { | ||
ex.printStackTrace(); | ||
} | ||
throw new IOException(ex); | ||
} | ||
return resultResources; | ||
} | ||
|
||
/** | ||
* Support for creating a runtime suitable for linking from the run-time | ||
* image. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
/* | ||
* Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved. | ||
* Copyright (c) 2014, 2025, Oracle and/or its affiliates. All rights reserved. | ||
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. | ||
* | ||
* This code is free software; you can redistribute it and/or modify it | ||
|
@@ -54,8 +54,8 @@ private ImageLocationWriter addAttribute(int kind, String value) { | |
} | ||
|
||
static ImageLocationWriter newLocation(String fullName, | ||
ImageStringsWriter strings, | ||
long contentOffset, long compressedSize, long uncompressedSize) { | ||
ImageStringsWriter strings, | ||
long contentOffset, long compressedSize, long uncompressedSize, int previewFlags) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrap or untab to avoid long lines. |
||
String moduleName = ""; | ||
String parentName = ""; | ||
String baseName; | ||
|
@@ -90,13 +90,14 @@ static ImageLocationWriter newLocation(String fullName, | |
} | ||
|
||
return new ImageLocationWriter(strings) | ||
.addAttribute(ATTRIBUTE_MODULE, moduleName) | ||
.addAttribute(ATTRIBUTE_PARENT, parentName) | ||
.addAttribute(ATTRIBUTE_BASE, baseName) | ||
.addAttribute(ATTRIBUTE_EXTENSION, extensionName) | ||
.addAttribute(ATTRIBUTE_OFFSET, contentOffset) | ||
.addAttribute(ATTRIBUTE_COMPRESSED, compressedSize) | ||
.addAttribute(ATTRIBUTE_UNCOMPRESSED, uncompressedSize); | ||
.addAttribute(ATTRIBUTE_MODULE, moduleName) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This got reformatted correctly to 8 indent because of the new entry. |
||
.addAttribute(ATTRIBUTE_PARENT, parentName) | ||
.addAttribute(ATTRIBUTE_BASE, baseName) | ||
.addAttribute(ATTRIBUTE_EXTENSION, extensionName) | ||
.addAttribute(ATTRIBUTE_OFFSET, contentOffset) | ||
.addAttribute(ATTRIBUTE_COMPRESSED, compressedSize) | ||
.addAttribute(ATTRIBUTE_UNCOMPRESSED, uncompressedSize) | ||
.addAttribute(ATTRIBUTE_PREVIEW_FLAGS, previewFlags); | ||
} | ||
|
||
@Override | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Introducing a flag bit that is carried around in lots of entries but applies to only one is overkill.
The simple test above for the first letter is cleaner and very localized.