-
Notifications
You must be signed in to change notification settings - Fork 665
[epilogue] Detailed trigger dependencies #8253
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: main
Are you sure you want to change the base?
Changes from 1 commit
7dfabed
f99b7d8
d4f1099
429eddf
2b13f61
e4ae20d
815480f
98f0f6f
eb28d17
39e05ea
37a2e52
3ad142f
539a41d
79b132c
bcc1eeb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -250,13 +250,32 @@ private void writeLoggerFile( | |
| out.println(); | ||
| } | ||
|
|
||
| // Static initializer block to load VarHandles and reflection fields | ||
| if (requiresVarHandles) { | ||
| out.println(" static {"); | ||
| // Generate static final LogMetadata fields for elements with @DependsOn annotations | ||
| List<Element> logMetadataElements = Stream.concat(loggableFields.stream(), loggableMethods.stream()) | ||
| .filter(element -> element.getAnnotation(DependsOn.class) != null || | ||
| element.getAnnotation(DependsOn.Container.class) != null) | ||
| .sorted(Comparator.comparing(e -> e.getSimpleName().toString())) | ||
| .collect(Collectors.toList()); | ||
|
|
||
| if (!logMetadataElements.isEmpty()) { | ||
| for (var element : logMetadataElements) { | ||
| // Generate static final LogMetadata field for caching | ||
| out.printf( | ||
| " // Cached LogMetadata for element %s with @DependsOn annotations%n", | ||
| element.getSimpleName()); | ||
| out.printf(" private static final LogMetadata %s;%n", logMetadataFieldName(element)); | ||
| } | ||
| out.println(); | ||
| } | ||
|
|
||
| out.println(" try {"); | ||
| // Static initializer block to load VarHandles and initialize LogMetadata fields | ||
| if (requiresVarHandles || !logMetadataElements.isEmpty()) { | ||
| out.println(" static {"); | ||
|
|
||
| out.println(" var rootLookup = MethodHandles.lookup();"); | ||
| if (requiresVarHandles) { | ||
| out.println(" try {"); | ||
| out.println(" var rootLookup = MethodHandles.lookup();"); | ||
| } | ||
|
|
||
| // Group private fields by class, then generate a private lookup for each class | ||
| // and a VarHandle for each field using that lookup. Sorting and then collecting into a | ||
|
|
@@ -294,11 +313,19 @@ private void writeLoggerFile( | |
| } | ||
| }); | ||
|
|
||
| out.println(" } catch (ReflectiveOperationException e) {"); | ||
| out.println( | ||
| " throw new RuntimeException(" | ||
| + "\"[EPILOGUE] Could not load private fields for logging!\", e);"); | ||
| out.println(" }"); | ||
| if (requiresVarHandles) { | ||
SamCarlberg marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| out.println(" } catch (ReflectiveOperationException e) {"); | ||
| out.println( | ||
| " throw new RuntimeException(" | ||
| + "\"[EPILOGUE] Could not load private fields for logging!\", e);"); | ||
| out.println(" }"); | ||
| } | ||
|
|
||
| // Initialize static final LogMetadata fields | ||
| for (var element : logMetadataElements) { | ||
| String metadataInitialization = generateLogMetadata(element, clazz, false); // Use inline construction for static field initialization | ||
| out.printf(" %s = %s;%n", logMetadataFieldName(element), metadataInitialization); | ||
| } | ||
|
|
||
| out.println(" }"); | ||
| out.println(); | ||
|
|
@@ -356,7 +383,7 @@ private void writeLoggerFile( | |
| // Generate log invocation for the main element | ||
| var metadata = generateLogMetadata(loggableElement, clazz); | ||
| if (metadata != null) { | ||
| var modifiedInvocation = logInvocation.replaceFirst("\\)$", ", " + metadata + ")"); | ||
| var modifiedInvocation = logInvocation.replaceFirst("\\)$", java.util.regex.Matcher.quoteReplacement(", " + metadata + ")")); | ||
|
||
| out.println(modifiedInvocation.indent(6).stripTrailing() + ";"); | ||
| } else { | ||
| out.println(logInvocation.indent(6).stripTrailing() + ";"); | ||
|
|
@@ -385,6 +412,18 @@ public static String varHandleName(VariableElement field) { | |
| .formatted(field.getEnclosingElement().toString().replace(".", "_"), field.getSimpleName()); | ||
| } | ||
|
|
||
| /** | ||
| * Generates the name of a static final LogMetadata field for the given element. The field name | ||
| * is guaranteed to be unique. | ||
| * | ||
| * @param element The element to generate a LogMetadata field for | ||
| * @return The name of the generated LogMetadata field | ||
| */ | ||
| public static String logMetadataFieldName(Element element) { | ||
| return "$metadata_%s_%s" | ||
| .formatted(element.getEnclosingElement().toString().replace(".", "_"), element.getSimpleName()); | ||
|
||
| } | ||
|
|
||
| private void collectLoggables( | ||
| TypeElement clazz, List<VariableElement> fields, List<ExecutableElement> methods) { | ||
| var config = clazz.getAnnotation(Logged.class); | ||
|
|
@@ -499,6 +538,18 @@ public Boolean visitIdentifier(IdentifierTree identifier, Void unused) { | |
| * @return LogMetadata containing dependency names, or null if no valid dependencies | ||
| */ | ||
| private String generateLogMetadata(Element element, TypeElement clazz) { | ||
| return generateLogMetadata(element, clazz, true); | ||
| } | ||
|
|
||
| /** | ||
| * Creates LogMetadata for dependencies of an element marked with @DependsOn annotations. | ||
| * | ||
| * @param element the element that has @DependsOn annotations | ||
| * @param clazz the class containing the element | ||
| * @param useStaticField if true, return static field reference; if false, return inline construction | ||
| * @return LogMetadata construction code or field reference, or null if no valid dependencies | ||
| */ | ||
| private String generateLogMetadata(Element element, TypeElement clazz, boolean useStaticField) { | ||
| // Check for @DependsOn annotations (both single and multiple via @DependsOn.Container) | ||
| List<DependsOn> dependencies = new ArrayList<>(); | ||
|
|
||
|
|
@@ -533,12 +584,16 @@ private String generateLogMetadata(Element element, TypeElement clazz) { | |
| return null; | ||
| } | ||
|
|
||
| // Create LogMetadata construction code | ||
| String dependencyList = validDependencyNames.stream() | ||
| .map(name -> "\"" + name + "\"") | ||
| .collect(java.util.stream.Collectors.joining(", ")); | ||
|
|
||
| return "new edu.wpi.first.epilogue.logging.LogMetadata(java.util.List.of(" + dependencyList + "))"; | ||
| if (useStaticField) { | ||
| // Return reference to static final field | ||
| return logMetadataFieldName(element); | ||
| } else { | ||
| // Create LogMetadata construction code for static field initialization | ||
| String dependencyList = validDependencyNames.stream() | ||
| .map(name -> "\"" + name + "\"") | ||
| .collect(java.util.stream.Collectors.joining(", ")); | ||
| return "new edu.wpi.first.epilogue.logging.LogMetadata(java.util.List.of(" + dependencyList + "))"; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2348,6 +2348,13 @@ public boolean isAtTarget() { | |
| import edu.wpi.first.epilogue.logging.LogMetadata; | ||
|
|
||
| public class ExampleLogger extends ClassSpecificLogger<Example> { | ||
| // Cached LogMetadata for element isAtTarget with @DependsOn annotations | ||
| private static final LogMetadata $metadata_edu_wpi_first_epilogue_Example_isAtTarget; | ||
|
|
||
| static { | ||
| $metadata_edu_wpi_first_epilogue_Example_isAtTarget = new edu.wpi.first.epilogue.logging.LogMetadata(java.util.List.of("position", "target", "tolerance")); | ||
|
||
| } | ||
|
|
||
| public ExampleLogger() { | ||
| super(Example.class); | ||
| } | ||
|
|
@@ -2358,7 +2365,7 @@ public void update(EpilogueBackend backend, Example object) { | |
| backend.log("position", object.position); | ||
| backend.log("target", object.target); | ||
| backend.log("tolerance", object.tolerance); | ||
| backend.log("isAtTarget", object.isAtTarget(), new edu.wpi.first.epilogue.logging.LogMetadata(java.util.List.of("position", "target", "tolerance"))); | ||
| backend.log("isAtTarget", object.isAtTarget(), $metadata_edu_wpi_first_epilogue_Example_isAtTarget); | ||
| } | ||
| } | ||
| } | ||
|
|
||
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.
Why can't we initialize the
LogMetadatacaches inline with the field declarations? (e.g.,private static final LogMetadata $metadata_edu_wpi_first_epilogue_Example_isAtTarget = new LogMetadata(List.of("position", "target", "tolerance"));)This would mean that we can remove the code in the
static {}initializer block, which would greater simplify the logic.