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

Parallel p4c builds recently broken? #1644

Closed
jafingerhut opened this issue Dec 27, 2018 · 6 comments · Fixed by #1645
Closed

Parallel p4c builds recently broken? #1644

jafingerhut opened this issue Dec 27, 2018 · 6 comments · Fixed by #1645
Labels
bug This behavior is unintended and should be fixed. fixed This topic is considered to be fixed.

Comments

@jafingerhut
Copy link
Contributor

jafingerhut commented Dec 27, 2018

I have used a script for building p4c for over a year now, consistently using the ability of make to do parallel builds using command line options like -j 4 to enable up to 4 parallel processes to be run by make.

Since some time around the latest version of p4c as of 2018-Dec-21, or perhaps starting 1 or 2 commits after that date, I get fairly consistent failures when using -j4, but it seems that I can get consistent build success if I use -j1, or no -j<number> argument at all.

Since there is an easy workaround, I do not consider this a high priority issue, but in case anyone who knows more about the p4c build process than I do wants this feature to work, I wanted to create an issue that perhaps it is not.

Below are the particular command line options and steps I have been using that cause the failure:

cd p4c
/bin/rm -fr build
mkdir build
cd build
cmake .. -DCMAKE_BUILD_TYPE=DEBUG
make -j4

I am using Ubuntu Linux 16.04 and/or 18.04 for my builds, and seeing similar parallel build issues with both.

@mihaibudiu
Copy link
Contributor

What is the error you get?
I can imagine two reasons: you run out of memory, or the scripts that run tests timeout. The two switch* tests are very long, and running one of these may timeout.

@jafingerhut
Copy link
Contributor Author

jafingerhut commented Dec 27, 2018

Trying out a few more variations, I noticed that I get a pretty consistent failure with one of my shell scripts that uses -j4, but another one with -j3 seems to successfully build pretty consistently. I have attached a file showing the error message I get with a -j4 run. I was running on a VM with 4 GBytes of RAM, and it never ran out of memory, as determined by running top while the failing build ran. At the time of the first compilation error messages coming out in that sample output, there was over 1 GByte of free RAM.
p4c-build-j4-error.txt

The failure occurs while building the executables. I am not running make check at all here.

@mihaibudiu
Copy link
Contributor

We've seen this bug before.
It has to do with the fact that p4parser.hpp is generated automatically. Some other compilation step attempts to use it before it is generated. So something is wrong with the dependences in the makefile.
This bug was introduced when the parser was refactored by @liujed .
He made some effort to fix it. @cc10512 suggested a fix which apparently didn't work.

@mihaibudiu
Copy link
Contributor

I reproduce here the error:

In file included from /home/jafinger/p4c/frontends/parsers/parserDriver.h:25:0,
                 from /home/jafinger/p4c/frontends/p4/parseAnnotations.h:21,
                 from /home/jafinger/p4c/frontends/p4/frontend.h:21,
                 from /home/jafinger/p4c/backends/bmv2/common/sharedActionSelectorCheck.h:23,
                 from /home/jafinger/p4c/backends/bmv2/common/control.h:30,
                 from /home/jafinger/p4c/backends/bmv2/common/control.cpp:17,
                 from /home/jafinger/p4c/build/backends/bmv2/unified_bmv2_backend_common_srcs_1.cpp:4:
/home/jafinger/p4c/frontends/parsers/p4/abstractP4Lexer.hpp:4:10: fatal error: frontends/parsers/p4/p4parser.hpp: No such file or directory
 #include "frontends/parsers/p4/p4parser.hpp"
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@liujed
Copy link
Member

liujed commented Dec 27, 2018

This results from a missing build dependency. I fixed a related issue in #1627, where I had to specify that the controlplane build target depends on frontend. The issue Andy is having is similar, and can likely be fixed by specifying that bmv2backend depends on frontend. I wouldn't be surprised if there are more missing dependencies lurking.

diff --git a/backends/bmv2/CMakeLists.txt b/backends/bmv2/CMakeLists.txt
index 13584809..6b8531c3 100644
--- a/backends/bmv2/CMakeLists.txt
+++ b/backends/bmv2/CMakeLists.txt
@@ -84,7 +84,7 @@ add_cpplint_files (${CMAKE_CURRENT_SOURCE_DIR} "${BMV2_BACKEND_COMMON_SRCS};${BM

 build_unified(BMV2_BACKEND_COMMON_SRCS)
 add_library(bmv2backend ${BMV2_BACKEND_COMMON_SRCS})
-add_dependencies(bmv2backend genIR)
+add_dependencies(bmv2backend genIR frontend)

 build_unified(BMV2_SIMPLE_SWiTCH_SRCS)
 add_executable(p4c-bm2-ss ${BMV2_SIMPLE_SWITCH_SRCS})

liujed added a commit that referenced this issue Dec 27, 2018
@mihaibudiu mihaibudiu added bug This behavior is unintended and should be fixed. fixed This topic is considered to be fixed. labels Dec 27, 2018
liujed added a commit that referenced this issue Dec 27, 2018
@jafingerhut
Copy link
Contributor Author

Wow, that was quick! I tried doing parallel builds with -j4 and -j6 after the fix, and found no problems. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This behavior is unintended and should be fixed. fixed This topic is considered to be fixed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants