bug(#3806): Add caching in UnphiMojo #4027

vl4ds4m · 2025-03-20T18:58:03Z

Closes: #3806
This PR embeds caching in UnphiMojo and enables cache test cases in UnphiMojoTest. But there is unresolved problem: these classes use irrelevant hash (FAKE_HASH) because i can't understand how to use relevant one, so i need help to resolve it.

github-actions · 2025-03-20T19:00:19Z

🚀 Performance Analysis

Test	Base Score	PR Score	Change	% Change	Unit	Mode
`benchmarks.XmirBench.xmirToEO`	174.755	154.202	-20.554	-11.76%	ms/op	Average Time
`benchmarks.XmirBench.xmirToPhi`	159.520	163.123	3.602	2.26%	ms/op	Average Time
`benchmarks.XmirBench.xmirToSaltyPhi`	168.963	140.127	-28.836	-17.07%	ms/op	Average Time

✅ Performance gain: benchmarks.XmirBench.xmirToEO is faster by 20.554 ms/op (11.76%)
⚠️ Performance loss: benchmarks.XmirBench.xmirToPhi is slower by 3.602 ms/op (2.26%)
✅ Performance gain: benchmarks.XmirBench.xmirToSaltyPhi is faster by 28.836 ms/op (17.07%)

vl4ds4m · 2025-03-20T21:57:39Z

I changed names and descriptions of footprint classes, which compare lastModified values of files, as older means the file is expired, not actual. Also, isAfter in former FpIfTargetOlder is isNotBefore now because a target file created with the source file at the same time is also actual.

vl4ds4m · 2025-03-20T22:00:00Z

@maxonfjvipon please, review it

maxonfjvipon · 2025-03-21T13:26:18Z

@vl4ds4m

your build is not clear, before review please make sure all the checks are passed
it looks like some of the changes are generated by AI. There's no any point to rename older to actual or isAfter to isNotBefore. So please remove them
Try to use empty string instead of hash for now

This reverts commit 7057a6b.

vl4ds4m · 2025-03-21T16:22:54Z

@maxonfjvipon I fixed build and reverted unnecessary changes, but empty string hash isn't valid for passing UnphiMojoTest tests with cache

maxonfjvipon · 2025-03-23T18:12:02Z

eo-maven-plugin/src/main/java/org/eolang/maven/FpIfTargetOlder.java

@@ -59,7 +59,7 @@ public final class FpIfTargetOlder extends FpEnvelope {
     * @throws IOException If fails to compare files
     */
    private static boolean isAfter(final Path first, final Path second) throws IOException {
-        return Files.getLastModifiedTime(first).toInstant().isAfter(
+        return !Files.getLastModifiedTime(first).toInstant().isBefore(


@vl4ds4m this change is unnecessary

maxonfjvipon · 2025-03-23T18:15:10Z

eo-maven-plugin/src/test/java/org/eolang/maven/UnphiMojoTest.java

    void usesCache(@Mktmp final Path temp) throws Exception {
        new Saved(
            "{⟦std ↦ Φ.org.eolang.io.stdout, y ↦ Φ.org.eolang.x⟧}",
            temp.resolve("target/eo/phi/std.phi")
        ).value();
-        final String hash = "123ZaRiFcHiK321";
+        final String hash = UnphiMojo.FAKE_HASH;


@vl4ds4m it's totally ok to slightly change the test after you introduce an implementation, let's try to use empty string here and in UnphiMojo because it doesn't depend on tojos

maxonfjvipon · 2025-03-23T18:17:53Z

eo-maven-plugin/src/test/java/org/eolang/maven/UnphiMojoTest.java

@@ -335,6 +329,11 @@ void invalidatesCache(@Mktmp final Path temp) throws Exception {
            "{⟦std ↦ Φ.org.eolang.io.stdout, y ↦ Φ.org.eolang.x⟧}",
            temp.resolve("target/eo/phi/std.phi")
        ).value();
+        MatcherAssert.assertThat(
+            "The cached file's last modified timestamp should be successfully reset",
+            cached.setLastModified(0L),


@vl4ds4m what's the reason to change this timestamp? Hard coded cached file was saved before the the original std.phi. So it'll be anyway younger

This reverts commit ef0e305.

vl4ds4m · 2025-03-24T07:47:30Z

eo-maven-plugin/src/main/java/org/eolang/maven/UnphiMojo.java

+    /**
+     * Hash directory for unphied files.
+     */
+    @SuppressWarnings("PMD.ImmutableField")
+    @Parameter(property = "eo.hash")
+    private String hash = "";
+


Introduced a field for hash with default empty value

@vl4ds4m it seems we have hash field in several mojos now, in ProbeMojo, in PullMojo and in UnphiMojo.
Let's make it only one by moving the field to the SafeMojo and make it protected so it'll look like:

abstract class SafeMojo { protected CommitHash hash = new ChConstant( new ChNarrow(new ChRemote(this.tag)) ); }

I think when it's done - it'll require some tests fixes. We're almost good to merge

vl4ds4m · 2025-03-24T07:50:48Z

eo-maven-plugin/src/test/java/org/eolang/maven/UnphiMojoTest.java

+        MatcherAssert.assertThat(
+            "The cached file's last modified timestamp should be strictly after the source one",
+            cached.setLastModified(cached.lastModified() + 1),
+            Matchers.is(true)
+        );


Changed a cached file timpestamp, because on some platforms these files have the same timestamp and the cached file isn't considered as valid

vl4ds4m · 2025-03-25T14:06:25Z

@maxonfjvipon please, take a look

maxonfjvipon · 2025-03-25T20:48:10Z

eo-maven-plugin/src/main/java/org/eolang/maven/UnphiMojo.java

+    /**
+     * Hash directory for unphied files.
+     */
+    @SuppressWarnings("PMD.ImmutableField")
+    @Parameter(property = "eo.hash")
+    private String hash = "";
+


@vl4ds4m it seems we have hash field in several mojos now, in ProbeMojo, in PullMojo and in UnphiMojo.
Let's make it only one by moving the field to the SafeMojo and make it protected so it'll look like:

abstract class SafeMojo { protected CommitHash hash = new ChConstant( new ChNarrow(new ChRemote(this.tag)) ); }

I think when it's done - it'll require some tests fixes. We're almost good to merge

maxonfjvipon

@vl4ds4m last change and we're good to merge

maxonfjvipon · 2025-03-26T08:53:42Z

eo-maven-plugin/src/main/java/org/eolang/maven/SafeMojo.java

+     * If not set, will be computed from {@code tag} field.
+     * @checkstyle VisibilityModifierCheck (5 lines)
+     */
+    protected CommitHash hash = new CommitHash.ChConstant(


@vl4ds4m let's use ChCached (caching decorator) instead of ChConst. This will allow not to call .value() explicitly on the next line, in the same way it's implemented in ProbeMojo

maxonfjvipon · 2025-03-28T11:58:26Z

@yegor256 please check

vl4ds4m added 2 commits March 20, 2025 20:58

Embed caching in UnphiMojo

91e08f6

Enable cache test cases for UnphiMojo with fake hash

feffddb

vl4ds4m added 3 commits March 20, 2025 22:03

Merge branch 'refs/heads/master' into 3806

3a3c7b5

Fix footprint compared file dates

7057a6b

Fix invalidatesCache test case

d2bac80

vl4ds4m added 4 commits March 21, 2025 18:53

Revert "Fix footprint compared file dates"

0958e3c

This reverts commit 7057a6b.

Change isAfter in FpIfTargetOlder.java

ef0e305

Remove todo tag

a30f400

Merge branch 'refs/heads/master' into 3806

fd1eb4e

maxonfjvipon suggested changes Mar 23, 2025

View reviewed changes

vl4ds4m added 6 commits March 24, 2025 09:47

Introduce hash parameter

dc555f9

Revert "Change isAfter in FpIfTargetOlder.java"

c362711

This reverts commit ef0e305.

Suppress warning on hash parameter

e4fc987

Change cache test cases

d3217a9

Add assertion on cached file timestamp

c287830

Merge branch 'refs/heads/master' into 3806

ffe7b84

vl4ds4m commented Mar 24, 2025

View reviewed changes

vl4ds4m requested a review from maxonfjvipon March 24, 2025 07:52

maxonfjvipon suggested changes Mar 25, 2025

View reviewed changes

vl4ds4m added 2 commits March 26, 2025 09:21

Merge branch 'refs/heads/master' into 3806

30fb52a

Move hash to SafeMojo

3679229

vl4ds4m requested a review from maxonfjvipon March 26, 2025 07:57

maxonfjvipon suggested changes Mar 26, 2025

View reviewed changes

Use ChCached for hash field

d0b67e2

vl4ds4m requested a review from maxonfjvipon March 26, 2025 09:35

maxonfjvipon approved these changes Mar 26, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bug(#3806): Add caching in UnphiMojo #4027

bug(#3806): Add caching in UnphiMojo #4027

vl4ds4m commented Mar 20, 2025

github-actions bot commented Mar 20, 2025 •

edited

Loading

vl4ds4m commented Mar 20, 2025

vl4ds4m commented Mar 20, 2025

maxonfjvipon commented Mar 21, 2025 •

edited

Loading

vl4ds4m commented Mar 21, 2025

maxonfjvipon Mar 23, 2025

maxonfjvipon Mar 23, 2025

maxonfjvipon Mar 23, 2025

vl4ds4m Mar 24, 2025

maxonfjvipon Mar 25, 2025

vl4ds4m Mar 24, 2025

vl4ds4m commented Mar 25, 2025

maxonfjvipon Mar 25, 2025

maxonfjvipon left a comment

maxonfjvipon Mar 26, 2025

maxonfjvipon commented Mar 28, 2025

bug(#3806): Add caching in UnphiMojo #4027

Are you sure you want to change the base?

bug(#3806): Add caching in UnphiMojo #4027

Conversation

vl4ds4m commented Mar 20, 2025

github-actions bot commented Mar 20, 2025 • edited Loading

🚀 Performance Analysis

vl4ds4m commented Mar 20, 2025

vl4ds4m commented Mar 20, 2025

maxonfjvipon commented Mar 21, 2025 • edited Loading

vl4ds4m commented Mar 21, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vl4ds4m commented Mar 25, 2025

Choose a reason for hiding this comment

maxonfjvipon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxonfjvipon commented Mar 28, 2025

github-actions bot commented Mar 20, 2025 •

edited

Loading

maxonfjvipon commented Mar 21, 2025 •

edited

Loading