Skip to content
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

kernel: use posix_spawn in iostreams if available #3513

Merged
merged 1 commit into from
Aug 30, 2019

Conversation

fingolfin
Copy link
Member

This should perform much better than fork() on Windows, and not worse than it on Unix variants.

Actually, to be safe, we probably will want to keep fork() as default. I mainly made posix_spawn default so that we can more easily test it.

This needs some more testing, esp. the part which changes the working directory.

In the meantime, I wonder if @alex-konovalov could test it on Windows to see if it helps with the performance of the children.tst file discussed in issue #3506

@fingolfin fingolfin added do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) topic: kernel release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Jun 19, 2019
@coveralls
Copy link

coveralls commented Jun 19, 2019

Coverage Status

Coverage increased (+0.001%) to 84.297% when pulling 6516d3f on fingolfin:mh/posix_spawn into 7302780 on gap-system:master.

@olexandr-konovalov
Copy link
Member

Will be happy to test on Windows - let me know when ready.

@fingolfin
Copy link
Member Author

@alex-konovalov while I plan to address @ChrisJefferson remarks, I think this is ready for testing on Windows right now -- the main questions are: (a) does it work / not crash at all, and (b) does children.tst run fast. If both are the case, it is worthwhile to improve this PR. If not, I might as well discard it (and can avoid the work to polish it).

@olexandr-konovalov
Copy link
Member

This does not seem to break anything Linux/macOS, but had no chance to try windows 32 and 64 yet.

@ChrisJefferson
Copy link
Contributor

ChrisJefferson commented Jun 22, 2019

Note this would also partially fix #3509, which would be nice -- although IO_fork would still be a problem when called explicitly.

@olexandr-konovalov
Copy link
Member

On windows 32 bit testinstall passes in cygwin shell, but then fails when it is performed on a Cygwin-free machine from the Windows command line shell:

testing: /proc/cygdrive/C/Users/jenkins/workspace/GAP-pull-request-windows-bui\
ld/GAP-pull-request-snapshot/tst/testinstall/kernel/streams.tst
A########> Diff in /proc/cygdrive/C/Users/jenkins/workspace/GAP-pull-request-wi\
ndows-build/GAP-pull-request-snapshot/tst/testinstall/kernel/streams.tst:11
# Input is:
LOG_TO(TmpName());
# Expected output:
true
# But found:
Error, LogTo: cannot log to /tmp/gaptempfile.r2y6Sb
########
########> Diff in /proc/cygdrive/C/Users/jenkins/workspace/GAP-pull-request-wi\
ndows-build/GAP-pull-request-snapshot/tst/testinstall/kernel/streams.tst:13
# Input is:
CLOSE_LOG_TO();
# Expected output:
true
# But found:
Error, LogTo: can not close the logfile
########
########> Diff in /proc/cygdrive/C/Users/jenkins/workspace/GAP-pull-request-wi\
ndows-build/GAP-pull-request-snapshot/tst/testinstall/kernel/streams.tst:28
# Input is:
INPUT_LOG_TO(TmpName());
# Expected output:
true
# But found:
Error, InputLogTo: cannot log to /tmp/gaptempfile.HJXXQy
########
########> Diff in /proc/cygdrive/C/Users/jenkins/workspace/GAP-pull-request-wi\
ndows-build/GAP-pull-request-snapshot/tst/testinstall/kernel/streams.tst:30
# Input is:
CLOSE_INPUT_LOG_TO();
# Expected output:
true
# But found:
Error, InputLogTo: can not close the logfile
########
########> Diff in /proc/cygdrive/C/Users/jenkins/workspace/GAP-pull-request-wi\
ndows-build/GAP-pull-request-snapshot/tst/testinstall/kernel/streams.tst:45
# Input is:
OUTPUT_LOG_TO(TmpName());
# Expected output:
true
# But found:
Error, OutputLogTo: cannot log to /tmp/gaptempfile.MxBLxv
########
########> Diff in /proc/cygdrive/C/Users/jenkins/workspace/GAP-pull-request-wi\
ndows-build/GAP-pull-request-snapshot/tst/testinstall/kernel/streams.tst:47
# Input is:
CLOSE_OUTPUT_LOG_TO();
# Expected output:
true
# But found:
Error, OutputLogTo: can not close the logfile
########

(link to the build - St Andrews only)

@olexandr-konovalov
Copy link
Member

Note that #3507 has been merged. As soon as other issues with this PR will be resolved, you can try to reenable that test on Windows to see if this helps with the performance of the children.tst.

@fingolfin
Copy link
Member Author

Unfortunately I currently have no way to debug these cygwin failures, which are somewhat baffling. I used to have an account on a machine supplied by @pedritomelenas but unfortunately that doesn't work anymore (I didn't use it for quite some time, too). So right now I have no good way to debug this; I guess I could try to once again set a Windows VM for this, but that's always quite some hassle and I just don't have the time and energy to deal with that right now.

@pedritomelenas
Copy link

@fingolfin I think you still have the account working there, write me on slack, probably the name of the machine changed...

@ChrisJefferson
Copy link
Contributor

What exactly is the problem on windows? I ran this on cygwin32 and 64, and seemed to work well. I still reduced the number of reps down to 10 as even posix_spawn isn't super-fast compared to forking on linux.

1 similar comment
@ChrisJefferson
Copy link
Contributor

What exactly is the problem on windows? I ran this on cygwin32 and 64, and seemed to work well. I still reduced the number of reps down to 10 as even posix_spawn isn't super-fast compared to forking on linux.

@fingolfin
Copy link
Member Author

@ChrisJefferson well @alex-konovalov reported test failures in 32 bit cygwin above (you can see the test output there, it involves LogTo). That's all I can say, as I can't test on cygwin myself right now.

@ChrisJefferson
Copy link
Contributor

ChrisJefferson commented Jul 16, 2019

Something weird is going on, that test error clearly doesn't line up with this PR -- the lines in question don't call TmpName, they call TmpNameAllArchs (if that's a good name for a function is another question, but that's what current master, and this PR, have in testinstall/kernel/streams.tst)

@codecov
Copy link

codecov bot commented Jul 19, 2019

Codecov Report

Merging #3513 into master will increase coverage by 0.87%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##           master    #3513      +/-   ##
==========================================
+ Coverage   84.55%   85.43%   +0.87%     
==========================================
  Files         698      699       +1     
  Lines      345122   346817    +1695     
==========================================
+ Hits       291805   296286    +4481     
+ Misses      53317    50531    -2786
Impacted Files Coverage Δ
src/iostream.c 61.71% <33.33%> (-3.44%) ⬇️
lib/csetpc.gi 42.69% <0%> (-45.51%) ⬇️
lib/autsr.gi 36.47% <0%> (-18.89%) ⬇️
lib/gpfpiso.gi 66.71% <0%> (-11.51%) ⬇️
lib/fieldfin.gi 78.3% <0%> (-10.2%) ⬇️
src/permutat.h 83.87% <0%> (-5.61%) ⬇️
lib/memory.gi 47.1% <0%> (-4.9%) ⬇️
src/sysmem.c 58.62% <0%> (-4.83%) ⬇️
lib/matobjplist.gi 48.62% <0%> (-1.74%) ⬇️
src/saveload.c 68.2% <0%> (-1.14%) ⬇️
... and 289 more

This should perform much better than fork() on Windows, and not worse
than it on Unix variants.
@olexandr-konovalov
Copy link
Member

I see an updated version of this PR - will test it on Windows again.

@olexandr-konovalov
Copy link
Member

olexandr-konovalov commented Aug 12, 2019

The new version seem to be better: the above mentioned errors disappeared. It shows only the diffs from #3332. Regarding the performance of children.tst, part of it is switched off in the test file if it's run on Windows. Either I need to login into the machine and run it manually there (will have to wait then) or you can edit this PR to reenable that test, and then I will rerun it via Jenkins - that will be more convenient for my workflow.

@fingolfin
Copy link
Member Author

@alex-konovalov so if it breaks nothing, but fixes some things, I am tempted to merge it. But of course it'd be nice to have some more testing first. So perhaps it should only be merged after stable-4.11 has been branched?

@olexandr-konovalov
Copy link
Member

Which "some more testing" do you have in mind?

@embray
Copy link
Contributor

embray commented Aug 21, 2019

Tested on 64-bit Cygwin; LGTM. Might be nice to have an option for this: Either a configure-time flag, or separate implementations using spawn() vs fork()/exec(), with one or the other as the default (probably spawn()). Not sure if it matters all that much though.

@wilfwilson
Copy link
Member

I believe that @DominikBernhardt also intends to test this in Windows, so let’s see whether he reports back soon.

I support merging this if it’s the case that it improves something and doesn’t make anything worse, even if it doesn’t fix everything.

@olexandr-konovalov
Copy link
Member

Ok. I already tested it on 32/64 bit linux / macos as part of the check I've mentioned above 10 days ago.

@pedritomelenas
Copy link

Tested in a fresh install on an external drive. gap.sh testinstall.g threw no errors; also as suggested by @alex-konovalov i tested gap.bat. I started gap.bat and did

dirs := [
  DirectoriesLibrary( "tst/testinstall" ),
];
TestDirectory( dirs, rec(exitGAP := false) );

No errors detected while testing either.

@olexandr-konovalov
Copy link
Member

olexandr-konovalov commented Aug 23, 2019

Thank you @pedritomelenas. If you have an opportunity, you can also test teststandard which will take maybe an hour or so.

Thank you also @embray for testing this. Note that it's fine to test it from cygwin shell, but for the approximation of a cygwin-free machine at least do like @pedritomelenas and start GAP using bin/gap.bat from Windows command line or file explorer. This will still be not a Cygwin-free machine, but the problems which may arise are usually fixed by finding and adding missing dll to the distribution.

@pedritomelenas
Copy link

On gap.sh:

gap> TestDirectory([DirectoriesLibrary("tst/teststandard"),],rec(exitGAP:=false));
Architecture: x86_64-unknown-cygwin-default64-kv6

testing: /home/pedro/tmp/gap/tst/teststandard/algext.tst
      94 ms (47 ms GC) and 8.13MB allocated for algext.tst
testing: /home/pedro/tmp/gap/tst/teststandard/arithlst.tst
   28000 ms (863 ms GC) and 3.34GB allocated for arithlst.tst
testing: /home/pedro/tmp/gap/tst/teststandard/ctblisoc.tst
    7734 ms (391 ms GC) and 1.39GB allocated for ctblisoc.tst
testing: /home/pedro/tmp/gap/tst/teststandard/ctblmoli.tst
   21875 ms (3819 ms GC) and 7.93GB allocated for ctblmoli.tst
testing: /home/pedro/tmp/gap/tst/teststandard/ctblsymm.tst
      63 ms (63 ms GC) and 1.15MB allocated for ctblsymm.tst
testing: /home/pedro/tmp/gap/tst/teststandard/direct_factors.tst
    3109 ms (232 ms GC) and 386MB allocated for direct_factors.tst
testing: /home/pedro/tmp/gap/tst/teststandard/grppcnrm.tst
   12094 ms (609 ms GC) and 1.52GB allocated for grppcnrm.tst
testing: /home/pedro/tmp/gap/tst/teststandard/grpprmcs.tst
    8766 ms (1737 ms GC) and 3.05GB allocated for grpprmcs.tst
testing: /home/pedro/tmp/gap/tst/teststandard/hash2.tst
    8234 ms (689 ms GC) and 1.18GB allocated for hash2.tst
testing: /home/pedro/tmp/gap/tst/teststandard/helptools.tst
     109 ms (62 ms GC) and 5.18MB allocated for helptools.tst
testing: /home/pedro/tmp/gap/tst/teststandard/hpc/alist.tst
#I  Test: File does not contain any tests!
       0 ms (0 ms GC) and 33.4KB allocated for hpc/alist.tst
testing: /home/pedro/tmp/gap/tst/teststandard/innerfunc.tst
    1094 ms (783 ms GC) and 211MB allocated for innerfunc.tst
testing: /home/pedro/tmp/gap/tst/teststandard/matrix.tst
    3750 ms (110 ms GC) and 126MB allocated for matrix.tst
testing: /home/pedro/tmp/gap/tst/teststandard/opers/AutomorphismGroup.tst
    9234 ms (1405 ms GC) and 4.38GB allocated for opers/AutomorphismGroup.tst
testing: /home/pedro/tmp/gap/tst/teststandard/opers/ComplementClassesRepresentatives.tst
    3891 ms (422 ms GC) and 1.54GB allocated for opers/ComplementClassesRepresentatives.tst
testing: /home/pedro/tmp/gap/tst/teststandard/opers/Normalizer.tst
    1844 ms (219 ms GC) and 414MB allocated for opers/Normalizer.tst
testing: /home/pedro/tmp/gap/tst/teststandard/opers/SemidirectDecompositions.tst
    1640 ms (140 ms GC) and 397MB allocated for opers/SemidirectDecompositions.tst
testing: /home/pedro/tmp/gap/tst/teststandard/opers/StructureDescription.tst
    2813 ms (411 ms GC) and 1.35GB allocated for opers/StructureDescription.tst
testing: /home/pedro/tmp/gap/tst/teststandard/permgrp.tst
   38844 ms (5753 ms GC) and 10.7GB allocated for permgrp.tst
testing: /home/pedro/tmp/gap/tst/teststandard/processes/children.tst
    3625 ms (0 ms GC) and 2.00MB allocated for processes/children.tst
testing: /home/pedro/tmp/gap/tst/teststandard/processes/streamio.tst
     109 ms (93 ms GC) and 116KB allocated for processes/streamio.tst
testing: /home/pedro/tmp/gap/tst/teststandard/reesmat.tst
    6422 ms (235 ms GC) and 529MB allocated for reesmat.tst
testing: /home/pedro/tmp/gap/tst/teststandard/simplegrpit.tst
     109 ms (94 ms GC) and 1.56MB allocated for simplegrpit.tst
testing: /home/pedro/tmp/gap/tst/teststandard/sort.tst
    3547 ms (109 ms GC) and 182MB allocated for sort.tst
testing: /home/pedro/tmp/gap/tst/teststandard/stabchain.tst
    1172 ms (219 ms GC) and 57.8MB allocated for stabchain.tst
testing: /home/pedro/tmp/gap/tst/teststandard/stablesort.tst
   10625 ms (218 ms GC) and 605MB allocated for stablesort.tst
testing: /home/pedro/tmp/gap/tst/teststandard/union.tst
    4563 ms (248 ms GC) and 842MB allocated for union.tst
testing: /home/pedro/tmp/gap/tst/teststandard/varnames.tst
     218 ms (93 ms GC) and 22.2MB allocated for varnames.tst
-----------------------------------
total    183578 ms (19064 ms GC) and 40.1GB allocated
              0 failures in 28 files

#I  No errors detected while testing

On gap.bat

gap> TestDirectory([DirectoriesLibrary("tst/teststandard"),],rec(exitGAP:=false));
Architecture: x86_64-unknown-cygwin-default64-kv6

testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/algext.tst
     109 ms (47 ms GC) and 8.12MB allocated for algext.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/arithlst.tst
   28391 ms (768 ms GC) and 3.34GB allocated for arithlst.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/ctblisoc.tst
    7484 ms (312 ms GC) and 1.39GB allocated for ctblisoc.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/ctblmoli.tst
   21500 ms (3339 ms GC) and 7.93GB allocated for ctblmoli.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/ctblsymm.tst
      47 ms (47 ms GC) and 1.14MB allocated for ctblsymm.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/direct_factors.tst
    3078 ms (156 ms GC) and 386MB allocated for direct_factors.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/grppcnrm.tst
   12422 ms (453 ms GC) and 1.52GB allocated for grppcnrm.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/grpprmcs.tst
    8875 ms (1892 ms GC) and 3.05GB allocated for grpprmcs.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/hash2.tst
    8766 ms (863 ms GC) and 1.18GB allocated for hash2.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/helptools.tst
     125 ms (63 ms GC) and 5.19MB allocated for helptools.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/hpc/alist.tst
#I  Test: File does not contain any tests!
       0 ms (0 ms GC) and 33.4KB allocated for hpc/alist.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/innerfunc.tst
    1156 ms (795 ms GC) and 211MB allocated for innerfunc.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/matrix.tst
    4109 ms (126 ms GC) and 126MB allocated for matrix.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/opers/AutomorphismGroup.tst
    9313 ms (1410 ms GC) and 4.38GB allocated for opers/AutomorphismGroup.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/opers/ComplementClassesRepresentatives.tst
    3890 ms (281 ms GC) and 1.54GB allocated for opers/ComplementClassesRepresentatives.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/opers/Normalizer.tst
    1860 ms (252 ms GC) and 414MB allocated for opers/Normalizer.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/opers/SemidirectDecompositions.tst
    1625 ms (77 ms GC) and 396MB allocated for opers/SemidirectDecompositions.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/opers/StructureDescription.tst
    2734 ms (393 ms GC) and 1.35GB allocated for opers/StructureDescription.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/permgrp.tst
   39375 ms (5620 ms GC) and 10.7GB allocated for permgrp.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/processes/children.tst
    3297 ms (0 ms GC) and 2.00MB allocated for processes/children.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/processes/streamio.tst
     125 ms (109 ms GC) and 117KB allocated for processes/streamio.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/reesmat.tst
    5953 ms (205 ms GC) and 529MB allocated for reesmat.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/simplegrpit.tst
     110 ms (94 ms GC) and 1.57MB allocated for simplegrpit.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/sort.tst
    3515 ms (78 ms GC) and 182MB allocated for sort.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/stabchain.tst
    1078 ms (188 ms GC) and 57.8MB allocated for stabchain.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/stablesort.tst
   10125 ms (126 ms GC) and 605MB allocated for stablesort.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/union.tst
    4500 ms (308 ms GC) and 842MB allocated for union.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/teststandard/varnames.tst
     188 ms (78 ms GC) and 22.2MB allocated for varnames.tst
-----------------------------------
total    183750 ms (18080 ms GC) and 40.1GB allocated
              0 failures in 28 files

#I  No errors detected while testing

In both cases children.tst took a long time.

@pedritomelenas
Copy link

Now testextra. With gap.sh

gap> TestDirectory([DirectoriesLibrary("tst/testextra"),],rec(exitGAP:=false)); Architecture: x86_64-unknown-cygwin-default64-kv6

testing: /home/pedro/tmp/gap/tst/testextra/ctblpope.tst
      93 ms (93 ms GC) and 75.2KB allocated for ctblpope.tst
testing: /home/pedro/tmp/gap/tst/testextra/grpauto.tst
 1028063 ms (60554 ms GC) and 179GB allocated for grpauto.tst
testing: /home/pedro/tmp/gap/tst/testextra/grplatt.tst
  188609 ms (15483 ms GC) and 31.3GB allocated for grplatt.tst
testing: /home/pedro/tmp/gap/tst/testextra/grpperm.tst
 7299641 ms (3258183 ms GC) and 3.29TB allocated for grpperm.tst
testing: /home/pedro/tmp/gap/tst/testextra/helpsys.tst
   25219 ms (10684 ms GC) and 1.74GB allocated for helpsys.tst
testing: /home/pedro/tmp/gap/tst/testextra/objmap.tst
   16281 ms (5875 ms GC) and 1.21GB allocated for objmap.tst
testing: /home/pedro/tmp/gap/tst/testextra/objset.tst
   10578 ms (2266 ms GC) and 390MB allocated for objset.tst
testing: /home/pedro/tmp/gap/tst/testextra/small_groups2.tst
   59250 ms (29376 ms GC) and 5.07GB allocated for small_groups2.tst
testing: /home/pedro/tmp/gap/tst/testextra/switch_obj.tst
  441297 ms (441234 ms GC) and 101MB allocated for switch_obj.tst
-----------------------------------
total   9069031 ms (3823748 ms GC) and 3.50TB allocated
              0 failures in 9 files

#I  No errors detected while testing

true

and with gap.bat

gap> TestDirectory([DirectoriesLibrary("tst/testextra"),],rec(exitGAP:=false));
Architecture: x86_64-unknown-cygwin-default64-kv6

testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/testextra/ctblpope.tst
     109 ms (109 ms GC) and 75.0KB allocated for ctblpope.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/testextra/grpauto.tst
 1035797 ms (65225 ms GC) and 179GB allocated for grpauto.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/testextra/grplatt.tst
  185234 ms (14957 ms GC) and 31.2GB allocated for grplatt.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/testextra/grpperm.tst
 6684219 ms (2703378 ms GC) and 3.08TB allocated for grpperm.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/testextra/helpsys.tst
   24969 ms (10578 ms GC) and 1.74GB allocated for helpsys.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/testextra/objmap.tst
   16016 ms (5718 ms GC) and 1.21GB allocated for objmap.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/testextra/objset.tst
   10203 ms (2092 ms GC) and 390MB allocated for objset.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/testextra/small_groups2.tst
   58203 ms (29031 ms GC) and 5.03GB allocated for small_groups2.tst
testing: /proc/cygdrive/D/cygwin-64/home/pedro/tmp/gap/tst/testextra/switch_obj.tst
  465562 ms (465546 ms GC) and 101MB allocated for switch_obj.tst
-----------------------------------
total   8480312 ms (3296634 ms GC) and 3.29TB allocated
              0 failures in 9 files

#I  No errors detected while testing

true

@olexandr-konovalov
Copy link
Member

Thank you very much, @pedritomelenas !

Copy link
Member

@olexandr-konovalov olexandr-konovalov left a comment

Choose a reason for hiding this comment

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

Approving on the grounds of the testing evidence. Dit not inspect the code.

@pedritomelenas
Copy link

With

dirs := [
  DirectoriesLibrary( "tst/teststandard" ),
  DirectoriesLibrary( "tst/testinstall" ),
  DirectoriesLibrary( "tst/testextra" ),
];
TestDirectory( dirs, rec(exitGAP :=false));

gap.bat crashes, which does not happen with gap.sh. If I add the options -A -x 80 -r -m 100m -o 1g -K 2g to gap.bat, then it does pass the tests.

If I remember well, this was already commented on slack. Probably a break should be more desirable here.

@fingolfin fingolfin changed the title WIP: kernel: use posix_spawn in iostreams if available kernel: use posix_spawn in iostreams if available Aug 30, 2019
@fingolfin fingolfin removed the do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) label Aug 30, 2019
@ChrisJefferson ChrisJefferson merged commit 6e34384 into gap-system:master Aug 30, 2019
@fingolfin fingolfin deleted the mh/posix_spawn branch August 30, 2019 17:16
@olexandr-konovalov olexandr-konovalov added this to the GAP 4.11.0 milestone Aug 30, 2019
@dimpase dimpase added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Feb 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: added PRs introducing changes that have since been mentioned in the release notes topic: kernel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants