Skip to content

IdempotencyTests is flaky on Windows #11885

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

Closed
smarter opened this issue Mar 24, 2021 · 13 comments · Fixed by #11944 or #11960
Closed

IdempotencyTests is flaky on Windows #11885

smarter opened this issue Mar 24, 2021 · 13 comments · Fixed by #11944 or #11960
Assignees
Milestone

Comments

@smarter
Copy link
Member

smarter commented Mar 24, 2021

Starting with the merge of #11210 (EDIT: actually long before that, see comments below), it looks like test_windows_full has been failing IdempotencyTests from time to time, from https://github.com/lampepfl/dotty/runs/2178741539:

Test 'CheckPosIdempotency from idempotency/check' failed with output:           
Idempotency test failed between out\idempotency\posIdempotency1\pos\i6507b\Test.tasty and out\idempotency\posIdempotency2\pos\i6507b\Test.tasty
Exception in thread "main" java.lang.reflect.InvocationTargetException
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at dotty.tools.vulpix.ChildJVMMain.runMain(ChildJVMMain.java:40)
	at dotty.tools.vulpix.ChildJVMMain.main(ChildJVMMain.java:47)
Caused by: java.lang.AssertionError: assertion failed: Failed 1 idempotency checks (out of 5116)
	at scala.runtime.Scala3RunTime$.assertFailed(Scala3RunTime.scala:8)
	at IdempotencyCheck$.checkIdempotency(IdempotencyCheck.scala:70)
	at Test$.main(CheckPosIdempotency.scala:4)
	at Test.main(CheckPosIdempotency.scala)
	... 6 more

@bishabosha could you investigate this (I've forwarded you the access information for the Windows machine if you want to investigate there)? /cc @liufengyun

@smarter smarter added this to the 3.0.0-RC2 milestone Mar 24, 2021
@griggt
Copy link
Contributor

griggt commented Mar 24, 2021

I think this test has been flaky for quite some time (at least many weeks). I've been meaning to trace back and see when it started. I want to say it was sometime around when the "expand non-transparent macros after typer" PR was merged, but my memory may be faulty.

@smarter
Copy link
Member Author

smarter commented Mar 24, 2021

Oh you're right, I didn't go back enough to see that, not @bishabosha's fault then :). /cc @nicolasstucki since he worked on both the PR you mentioned and idempotency tests.

@griggt
Copy link
Contributor

griggt commented Mar 24, 2021

Here's an example of it failing during the nightly build of Feb 9 (the night after #9984 was merged):

https://github.com/lampepfl/dotty/runs/1868294322?check_suite_focus=true#step:3:784

@griggt
Copy link
Contributor

griggt commented Mar 24, 2021

Incidentally I'll open a PR for #8734 sometime soon (maybe after the RC release is done), which will draw attention to these things right away.

@michelou
Copy link
Contributor

michelou commented Mar 24, 2021

I think this test has been flaky for quite some time (at least many weeks). I've been meaning to trace back and see when it started. I want to say it was sometime around when the "expand non-transparent macros after typer" PR was merged, but my memory may be faulty.

@griggt I confirm your observation. Here are the 3 failing tests I currently have on my Win10 machine :

[...]
[error] Failed tests:
[error]         dotty.tools.dotc.FromTastyTests
[error]         dotty.tools.dotc.IdempotencyTests
[error]         dotty.tools.dotc.classpath.MultiReleaseJarTest

PS. I build at the moment 16 snapshot distributions (tar.gz+zip) on a little used Win10 machine (~30 min build time each) : BellSoft 8+11, Corretto 8+11, DCEVM 11, OpenJ9 8+11, OpenJDK 8+11+17, RedHat 8+11, SapMachine 11+17 and Zulu 8+11.

@griggt
Copy link
Contributor

griggt commented Mar 25, 2021

The discrepancy seems to be in the positions section:

--- out/idempotency/posIdempotency1/debug-idemp/i6507b/tasty.txt	2021-03-25 12:16:34.354135344 -0700
+++ out/idempotency/posIdempotency2/debug-idemp/i6507b/tasty.txt	2021-03-25 12:16:53.166135344 -0700
@@ -264,7 +264,7 @@
    436:         STRINGconst 56 [tests/debug-idemp/i6507b.scala]
    438:
 
- 291 position bytes:
+ 288 position bytes:
    lines: 11
    line sizes: 26, 0, 13, 79, 36, 56, 3, 0, 28, 1, 0
    positions:
@@ -337,7 +337,7 @@
        339: 197 .. 215
        341: 193 .. 193
        347: 183 .. 196
-       350: 200 .. 215
+       350: 183 .. 183
        353: 183 .. 196
        357: 200 .. 215
        364: 197 .. 215
   284:       INLINED(130)
   287:         BLOCK(124)
   289:           TYPED(112)
   291:             APPLY(107)
   293:               TYPEAPPLY(102)
   295:                 SELECTin(95) 41 [*:[Signed Signature(List(2, java.lang.Object),scala.Product) @*:]]
   298:                   INLINED(89)
   300:                     BLOCK(83)
   302:                       TYPED(71)
   304:                         APPLY(66)
   306:                           TYPEAPPLY(61)
   308:                             SELECTin(54) 41 [*:[Signed Signature(List(2, java.lang.Object),scala.Product) @*:]]
   311:                               INLINED(48)
   313:                                 BLOCK(42)
   315:                                   TYPED(30)
   317:                                     APPLY(25)
   319:                                       TYPEAPPLY(20)
   321:                                         SELECTin(13) 41 [*:[Signed Signature(List(2, java.lang.Object),scala.Product) @*:]]
   324:                                           INLINED(7)
   326:                                             SHAREDterm 151
   329:                                             IDENTtpt 6 [Test[ModuleClass]]
   331:                                               SHAREDtype 18
   333:                                           SHAREDtype 241
   336:                                         INTconst 98
   339:                                         SHAREDtype 112
   341:                                       TERMREFdirect 347
   344:                                     SHAREDterm 253
   347:                                   VALDEF(8) 44 [x[Unique $ 1]]
   350:                                     SHAREDtype 336
   353:                                     INTconst 98
   356:                                     SYNTHETIC

The output files from this test run: i11885-output.zip

@odersky odersky modified the milestones: 3.0.0-RC2, 3.0.x Mar 29, 2021
@liufengyun liufengyun self-assigned this Mar 29, 2021
liufengyun added a commit to dotty-staging/dotty that referenced this issue Mar 30, 2021
liufengyun added a commit to dotty-staging/dotty that referenced this issue Mar 30, 2021
liufengyun added a commit to dotty-staging/dotty that referenced this issue Mar 30, 2021
liufengyun added a commit that referenced this issue Mar 30, 2021
Fix #11885: disable flaky idempotency test on Windows
@liufengyun
Copy link
Contributor

Reopen, because #11944 is a temporary fix.

@michelou
Copy link
Contributor

@liufengyun Assuming the change to stopAfter in file Run.scala and to opt in file IdempotencyTests.scala does solve the issue on Windows, how do you explain it was observed on Windows only ?!

@liufengyun
Copy link
Contributor

@liufengyun Assuming the change to stopAfter in file Run.scala and to opt in file IdempotencyTests.scala does solve the issue on Windows, how do you explain it was observed on Windows only ?!

It's related to the parallelism in Pickler

      if !Pickler.ParallelPickling || ctx.settings.YtestPickler.value then force()

The problem is that sometimes on Windows, it's slower -- and the later phase Inlining can change the positions of the trees to be pickled, thus non-determinism.

liufengyun added a commit to dotty-staging/dotty that referenced this issue Mar 31, 2021
The cause of the problem is that we use parallelism in Pickler:

      if !Pickler.ParallelPickling || ctx.settings.YtestPickler.value then force()

Sometimes on Windows, the futures run a little slower, and the later
phase `Inlining` can change the positions of the trees to be pickled,
thus non-determinism.
liufengyun added a commit to dotty-staging/dotty that referenced this issue Mar 31, 2021
The cause of the problem is that we use parallelism in Pickler:

      if !Pickler.ParallelPickling || ctx.settings.YtestPickler.value then force()

Sometimes on Windows, the futures run a little slower, and the later
phase `Inlining` can change the positions of the trees to be pickled,
thus non-determinism.
liufengyun added a commit to dotty-staging/dotty that referenced this issue Mar 31, 2021
The cause of the problem is that we use parallelism in Pickler:

      if !Pickler.ParallelPickling || ctx.settings.YtestPickler.value then force()

Sometimes on Windows, the futures run a little slower, and the later
phase `Inlining` can change the positions of the trees to be pickled,
thus non-determinism.
liufengyun added a commit to dotty-staging/dotty that referenced this issue Mar 31, 2021
The cause of the problem is that we use parallelism in Pickler:

      if !Pickler.ParallelPickling || ctx.settings.YtestPickler.value then force()

Sometimes on Windows, the futures run a little slower, and the later
phase `Inlining` can change the positions of the trees to be pickled,
thus non-determinism.
liufengyun added a commit to dotty-staging/dotty that referenced this issue Mar 31, 2021
The cause of the problem is that we use parallelism in Pickler:

      if !Pickler.ParallelPickling || ctx.settings.YtestPickler.value then force()

Sometimes on Windows, the futures run a little slower, and the later
phase `Inlining` can change the positions of the trees to be pickled,
thus non-determinism.
liufengyun added a commit to dotty-staging/dotty that referenced this issue Mar 31, 2021
The cause of the problem is that we use parallelism in Pickler:

      if !Pickler.ParallelPickling || ctx.settings.YtestPickler.value then force()

Sometimes on Windows, the futures run a little slower, and the later
phase `Inlining` can change the positions of the trees to be pickled,
thus non-determinism.
michelou pushed a commit to michelou/scala3 that referenced this issue Apr 6, 2021
@griggt
Copy link
Contributor

griggt commented Apr 16, 2021

This flaky test just failed again. I thought it was disabled?

https://github.com/lampepfl/dotty/runs/2360396782?check_suite_focus=true#step:4:594

@liufengyun
Copy link
Contributor

This flaky test just failed again. I thought it was disabled?

https://github.com/lampepfl/dotty/runs/2360396782?check_suite_focus=true#step:4:594

It looks like the test filter does not work well due to the difference in path separators on Windows (#11950). I'll have a look.

liufengyun added a commit to dotty-staging/dotty that referenced this issue Apr 17, 2021
liufengyun added a commit to dotty-staging/dotty that referenced this issue Apr 19, 2021
The cause of the problem is that we use parallelism in Pickler:

      if !Pickler.ParallelPickling || ctx.settings.YtestPickler.value then force()

Sometimes on Windows, the futures run a little slower, and the later
phase `Inlining` can change the positions of the trees to be pickled,
thus non-determinism.

For the Dotty project, the statistics is as follows:

- Before: ntrees = 5331539
- After:  ntrees = 5334075

Performance-wise, this should be better than synchronizing the pickling tasks.
@griggt
Copy link
Contributor

griggt commented Apr 21, 2021

@liufengyun
Copy link
Contributor

Maybe the exclude filter is missing .scala suffix?

Thanks for the good catch, somehow I thought it's a directory 😕 (#12164)

@anatoliykmetyuk anatoliykmetyuk modified the milestones: 3.0.x, 3.0.1-RC1 May 12, 2021
smarter pushed a commit to dotty-staging/dotty that referenced this issue May 14, 2021
The cause of the problem is that we use parallelism in Pickler:

      if !Pickler.ParallelPickling || ctx.settings.YtestPickler.value then force()

Sometimes on Windows, the futures run a little slower, and the later
phase `Inlining` can change the positions of the trees to be pickled,
thus non-determinism.

For the Dotty project, the statistics is as follows:

- Before: ntrees = 5331539
- After:  ntrees = 5334075

Performance-wise, this should be better than synchronizing the pickling tasks.
liufengyun added a commit to dotty-staging/dotty that referenced this issue May 25, 2021
The cause of the problem is that we use parallelism in Pickler:

      if !Pickler.ParallelPickling || ctx.settings.YtestPickler.value then force()

Sometimes on Windows, the futures run a little slower, and the later
phase `Inlining` can change the positions of the trees to be pickled,
thus non-determinism.

For the Dotty project, the statistics is as follows:

- Before: ntrees = 5331539
- After:  ntrees = 5334075

Performance-wise, this should be better than synchronizing the pickling tasks.
liufengyun added a commit that referenced this issue May 25, 2021
Fix #11885: Always clone trees if position already set
@Kordyjan Kordyjan modified the milestones: 3.0.1-RC1, 3.0.1 Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment