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

Get more of the tests passing for FreeBSD #3197

Merged
merged 10 commits into from Sep 10, 2019
Merged

Get more of the tests passing for FreeBSD #3197

merged 10 commits into from Sep 10, 2019

Conversation

ghost
Copy link

@ghost ghost commented Sep 8, 2019

These changes were necessary to get all of the tests, both with -Dskip-release and not, passing for FreeBSD.

This should hopefully check the respecting box in issue #1759 and get the wheels rolling.

@andrewrk
Copy link
Member

andrewrk commented Sep 9, 2019

Nice work!

Want to try enabling more tests in the FreeBSD CI by adding this to your PR?

--- a/ci/srht/freebsd_script
+++ b/ci/srht/freebsd_script
@@ -18,14 +18,11 @@ cd build
 cmake .. -DCMAKE_BUILD_TYPE=Release -DCMAKE_PREFIX_PATH=$PREFIX -DCMAKE_INSTALL_PREFIX=$(pwd)/release -DZIG_STATIC=ON
 make $JOBS install
 
-# TODO test everything. right now it's skipping stuff including docs
-# because for some reason @cImport is failing on the CI server.
-release/bin/zig build --build-file ../build.zig test-behavior -Dskip-release
+release/bin/zig build test
 
 if [ -f ~/.s3cfg ]; then
   mv ../LICENSE release/
-  # TODO re-enable this
-  #mv ../zig-cache/langref.html release/
+  mv ../zig-cache/langref.html release/
   mv release/bin/zig release/
   rmdir release/bin

@ghost
Copy link
Author

ghost commented Sep 9, 2019

Your patch has been applied 😸

@mikdusan
Copy link
Member

mikdusan commented Sep 9, 2019

apparently builds.sr.ht vm is 2GB and on my own vm is set at 6GB and consistently 4.1GB to 4.7GB proc is showing up:

zig build test

  PID USERNAME    THR PRI NICE   SIZE    RES STATE    C   TIME    WCPU COMMAND
 6388 mike          1 103    0  4157M  4058M CPU3     3   1:06  98.91% zig
 5574 mike          1  20    0    13M  2908K CPU1     1   0:01   0.01% top

@andrewrk
Copy link
Member

andrewrk commented Sep 9, 2019

we have 2 courses of action here:

  • try to reduce memory usage of stage1
  • try to convince @ddevault to give us 6 GiB RAM

For reducing memory usage of stage1, it's a lot of work. We need a framework for understanding where the memory is, to tell us what efforts would be rewarded. And then we have to be careful to not reduce the debuggability of stage1. E.g. if we tried to squeeze 3 flags into every 8 byte aligned pointer, that would save memory, but would probably cause bugs. Perhaps such tricks will be tried in the self-hosted compiler, but for stage1 the real priority is debuggability & maintainability as the language evolves.

For convincing Drew, maybe I could offer to double my SourceHut donations? :-)

@ghost
Copy link
Author

ghost commented Sep 9, 2019

Would using the heap profiler/checker from gperftools would help? https://github.com/gperftools/gperftools

@andrewrk
Copy link
Member

andrewrk commented Sep 9, 2019

Would using the heap profiler/checker from gperftools would help? https://github.com/gperftools/gperftools

Maybe! I've never tried that before.

In the meantime, here's something we can do, to merge your PR. Instead of:

release/bin/zig build test

which does all the tests. Let's see how many we can enable. It looks like the most memory used is in the std lib tests. Here's a way to run a subset:

$ ./zig build --help
Usage: /home/andy/dev/zig/build/zig build [steps] [options]

Steps:
  install (default)      Copy build artifacts to prefix path
  uninstall              Remove build artifacts from prefix path
  docs                   Build documentation
  test                   Run all the tests
  libuserland            Build the userland compiler library for use in stage1
  test-stage2            Run the stage2 compiler tests
  test-fmt               Run zig fmt against build.zig to make sure it works
  test-behavior          Run the behavior tests
  test-std               Run the standard library tests
  test-compiler-rt       Run the compiler_rt tests
  test-compare-output    Run the compare output tests
  test-standalone        Run the standalone tests
  test-stack-traces      Run the stack trace tests
  test-cli               Test the command line interface
  test-asm-link          Run the assemble and link tests
  test-runtime-safety    Run the runtime safety tests
  test-translate-c       Run the C transation tests
  test-gen-h             Run the C header file generation tests
  test-compile-errors    Run the compile error tests

General Options:
  --help                 Print this help and exit
  --verbose              Print commands before executing them
  --prefix [path]        Override default install prefix
  --search-prefix [path] Add a path to look for binaries, libraries, headers

Project-Specific Options:
  -Drelease=[bool]       create a release build (ReleaseFast)
  -Doutput-dir=[string]  For libuserland step, where to put the output
  -Dskip-release=[bool]  Main test suite skips release builds
  -Dskip-release-small=[bool] Main test suite skips release-small builds
  -Dskip-release-fast=[bool] Main test suite skips release-fast builds
  -Dskip-release-safe=[bool] Main test suite skips release-safe builds
  -Dskip-non-native=[bool] Main test suite skips non-native builds
  -Dskip-self-hosted=[bool] Main test suite skips building self hosted compiler
  -Dlib-files-only=[bool] Only install library files
  -Dtest-filter=[string] Skip tests that do not match filter

Advanced Options:
  --build-file [file]      Override path to build.zig
  --cache-dir [path]       Override path to zig cache directory
  --override-std-dir [arg] Override path to Zig standard library
  --override-lib-dir [arg] Override path to Zig lib directory
  --verbose-tokenize       Enable compiler debug output for tokenization
  --verbose-ast            Enable compiler debug output for parsing into an AST
  --verbose-link           Enable compiler debug output for linking
  --verbose-ir             Enable compiler debug output for Zig IR
  --verbose-llvm-ir        Enable compiler debug output for LLVM IR
  --verbose-cimport        Enable compiler debug output for C imports
  --verbose-cc             Enable compiler debug output for C compilation

All these "steps" can be run independently. So we can try something like this:

release/bin/zig build test-fmt
release/bin/zig build test-behavior
# release/bin/zig build test-std (commented out)
release/bin/zig build test-compiler-rt
release/bin/zig build test-compare-output
release/bin/zig build test-standalone
release/bin/zig build test-stack-traces
release/bin/zig build test-cli
release/bin/zig build test-asm-link
release/bin/zig build test-runtime-safety
release/bin/zig build test-translate-c
release/bin/zig build test-gen-h
release/bin/zig build test-compile-errors
release/bin/zig build docs

Maybe more have to be commented out; maybe not. But currently we're only doing test-behavior -Dskip-release and I'm sure with your improvements, we can get more coverage.

@ddevault
Copy link

ddevault commented Sep 9, 2019

Sorry, 6G isn't happening.

@andrewrk
Copy link
Member

andrewrk commented Sep 9, 2019

Another approach would be switching to Azure for FreeBSD. We already use Azure for Linux, macOS, and Windows, and they provide enough RAM for our purposes. Here's a guide on FreeBSD. It looks like they do have FreeBSD 12 available. I'll look into this soon.

I really would like to spend more time evolving & stabilizing the language right now and less time optimizing the stage1 compiler, so I'm inclined to look into this approach rather than the other one.

@mikdusan
Copy link
Member

mikdusan commented Sep 9, 2019

@mikdusan

This comment has been minimized.

@ghost
Copy link
Author

ghost commented Sep 10, 2019

After #3203 has been merged, should I rebase my PR to pick up more possible failures?

@andrewrk
Copy link
Member

After #3203 has been merged, should I rebase my PR to pick up more possible failures?

Yes, if you don't mind, try doing my suggestion in #3197 (comment) and let's see how many of these tests we can enable.

@andrewrk
Copy link
Member

I see 2 things from this build log:

  • error: C import failed: out of memory which means test-compare-output will have to be disabled until this can be resolved
  • I don't think this was rebased on a recent enough master, because it appears to not be running @mikdusan's new code. I see "skipping execution because it is non-native" for all the behavior tests, but I would expect to see a few of them run natively on FreeBSD.

@andrewrk andrewrk changed the title Get all the tests passing for FreeBSD Get more of the tests passing for FreeBSD Sep 10, 2019
@ghost
Copy link
Author

ghost commented Sep 10, 2019

Sorry about that Andrew. I'm without my morning coffee... I got both the extra test disabled and rebased on upstream master.

@ghost
Copy link
Author

ghost commented Sep 10, 2019

Well looks like I found a genuine test failure as it doesn't look like it's Out Of Memory. I'll see if I can fix that.

@andrewrk
Copy link
Member

I'm sorry that the error message didn't contain stderr, since that would have been helpful, but I used sr.ht's feature to log into the VM after a failed build and poked around, and saw this:

[build@build /usr/home/build/zig/build]$ ./release/bin/zig translate-c ../zig-cache/source.h 
unable to parse C file: out of memory

@andrewrk
Copy link
Member

For take5 (if necessary) would you mind adding a comment for every commented out test explaining why the test is disabled? I believe it's the same for all of them, but let's duplicate the information for every disabled test. Example:

# This test is disabled because it triggers "out of memory" on the sr.ht CI service.
# See https://github.com/ziglang/zig/issues/3210
# release/bin/zig build test-translate-c 

@ghost
Copy link
Author

ghost commented Sep 10, 2019

Phew 😤

@andrewrk
Copy link
Member

You're going to have to revert the langref.html changes too, because that is produced by the docs step.

@andrewrk
Copy link
Member

Don't despair! You have succeeded in getting significantly more tests passing, as well as making it abundantly clear what the next steps are to get to full CI test coverage for FreeBSD. This is a meaningful improvement.

@ghost
Copy link
Author

ghost commented Sep 10, 2019

Thank you Andrew, I'm glad to help. I think I deserve some breakfast after going through each take. 🍳

@andrewrk andrewrk merged commit a165cc0 into ziglang:master Sep 10, 2019
@ghost ghost deleted the freebsd-passed-tests branch September 10, 2019 21:20
@dch
Copy link

dch commented Dec 12, 2019

I can provide access to a system with enough ram. ping me (dch) on irc to discuss. we should be able to hook it into sr.ht API as well I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants