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

Upgrading to grpc 1.39.0 makes our cmake-install build too slow #7052

Closed
devjgm opened this issue Jul 23, 2021 · 8 comments · Fixed by #7061
Closed

Upgrading to grpc 1.39.0 makes our cmake-install build too slow #7052

devjgm opened this issue Jul 23, 2021 · 8 comments · Fixed by #7061
Labels
type: cleanup An internal cleanup or hygiene concern.

Comments

@devjgm
Copy link
Contributor

devjgm commented Jul 23, 2021

Step #3: GCB: https://console.cloud.google.com/cloud-build/builds;region=us-east1/16a86463-87c8-4b20-ae98-bc8913b3de7a;tab=detail?project=cloud-cpp-testing-resources
Step #3: Raw: https://storage.googleapis.com/cloud-cpp-community-publiclogs/logs/google-cloud-cpp/7046/2fcd9ef25cf6c6221ea8af73def9e9d0ebe05ea5/fedora-34-cmake-install/log-16a86463-87c8-4b20-ae98-bc8913b3de7a.txt

We upgraded grpc from 1.38.1 to 1.39.0 and our cmake-install build became unusably slow, so we rolled it back. To roll this change forward git revert ba41d5a99818922b09eee5da148786c1d68c17f5 (ba41d5a)

The slow line is:

pkg_check_modules(${module} IMPORTED_TARGET REQUIRED ${module})

The line is slow when checking a module that links with grpc, like google_cloud_cpp_grpc_utils

I suspect there may be some change in this version of grpc to its pkg-config files. Maybe it's also exposing a bug in pkg-config itself, maybe similar to #6846

NOTE: We need to figure this out because we need to be able to upgrade gRPC

@devjgm devjgm added the type: cleanup An internal cleanup or hygiene concern. label Jul 23, 2021
@devjgm
Copy link
Contributor Author

devjgm commented Jul 23, 2021

@veblush FYI

@devjgm
Copy link
Contributor Author

devjgm commented Jul 23, 2021

We see this in a docker image w/ the following info:

Step #3: ====================
Step #3: |   Machine Info   |
Step #3: ====================
Step #3:      host: 2021-07-23 14:50:49+00:00
Step #3:    google: 2021-07-23 14:50:49+00:00
Step #3:    kernel: #49~18.04.1-Ubuntu SMP Fri Jun 18 21:33:55 UTC 2021
Step #3:        os: PRETTY_NAME="Fedora 34 (Container Image)"
Step #3:     nproc: 32
Step #3:       mem: 125.81 GiB
Step #3:      term: dumb
Step #3:       gcc: gcc (GCC) 11.1.1 20210531 (Red Hat 11.1.1-3)
Step #3:     clang: clang version 12.0.0 (Fedora 12.0.0-2.fc34)
Step #3:        cc: cc (GCC) 11.1.1 20210531 (Red Hat 11.1.1-3)

@devjgm
Copy link
Contributor Author

devjgm commented Jul 23, 2021

I just discovered that fedora uses pkgconf not pkg-config. The project is https://github.com/pkgconf/pkgconf. Fedora 34 uses pkgconf 1.7.3.

@devjgm
Copy link
Contributor Author

devjgm commented Jul 23, 2021

I suspect the reason for the slowness is that grpc moved a bunch of linker flags from the pkg-config Libs: line to Requires:.

grpc 1.38.1

$ cat /usr/local/lib/pkgconfig/grpc++.pc
prefix=/usr/local
exec_prefix=${prefix}
includedir=${prefix}/include
libdir=${exec_prefix}/lib

Name: gRPC++
Description: C++ wrapper for gRPC
Version: 1.35.0
Cflags: -I${includedir}
Requires: grpc
Libs: -L${libdir} -lgrpc++ -labsl_raw_hash_set -labsl_hashtablez_sampler -labsl_exponential_biased -labsl_hash -labsl_city -labsl_statusor -labsl_bad_variant_access -labsl_status -labsl_cord -labsl_str_format_internal -labsl_synchronization -labsl_graphcycles_internal -labsl_symbolize -labsl_demangle_internal -labsl_stacktrace -labsl_debugging_internal -labsl_malloc_internal -labsl_time -labsl_time_zone -labsl_civil_time -labsl_strings -labsl_strings_internal -labsl_throw_delegate -labsl_int128 -labsl_base -labsl_spinlock_wait -labsl_bad_optional_access -labsl_raw_logging_internal -labsl_log_severity
Libs.private:

grpc 1.39.0

$ cat /usr/local/lib/pkgconfig/grpc++.pc
prefix=/usr/local
exec_prefix=${prefix}
includedir=${prefix}/include
libdir=${exec_prefix}/lib

Name: gRPC++
Description: C++ wrapper for gRPC
Version: 1.39.0
Cflags: -I${includedir}
Requires: grpc absl_base absl_bind_front absl_flat_hash_map absl_inlined_vector absl_memory absl_optional absl_status absl_statusor absl_str_format absl_strings absl_synchronization absl_time
Libs: -L${libdir} -lgrpc++
Libs.private: 

NOTE: I do not think that this is a gRPC bug. But it may have exposed a bug, or inefficiency, in the pkg-config implementation where it's just slow on packages with lots of "requires" libraries.

@devjgm
Copy link
Contributor Author

devjgm commented Jul 24, 2021

FWIW, here's the gRPC PR that introduced this change: grpc/grpc#25840

AFAICT, gRPC seems to be using pkg-config correctly. I don't see anything wrong that gRPC is doing wrt pkg-config. This just may be a bug or limitation of pkg-config itself.

  1. Is there a "fix" that gRPC could/should make? I don't see it.
  2. Is there a "fix" or workaround that we could make?
  3. Are we using pkg_check_modules correctly? Should we be using that?

At this point, I'm suspecting that pkg-config simply does not scale well.

@devjgm
Copy link
Contributor Author

devjgm commented Jul 26, 2021

@derekmauro and @dinor7c JFYI

Summary: Cloud C++ upgraded to grpc 1.39.0 and our builds that use pkg-config got really slow. We had to roll back. The issue is that grpc listed all of its pkg-config deps (which are mostly Abseil deps) in its Requires: line. For some reason, this makes pkg-config (actually pkgconf on fedora) quite slow. I think this is actually a pkg-config performance problem; I do not think grpc nor abseil are necessarily doing anything wrong.

@devjgm
Copy link
Contributor Author

devjgm commented Jul 26, 2021

Fedora 34 by default uses pkgconf 1.7.3 from https://github.com/pkgconf/pkgconf. This version shows the problem in this issue.

docker:fedora-34$ time pkgconf google_cloud_cpp_grpc_utils --libs-only-l --static
...
real    0m5.019s

I compiled pkg-config 0.29.0 from https://pkgconfig.freedesktop.org/releases/pkg-config-0.29.tar.gz , and this version does NOT show the problem.

docker:fedora-34$ time ./pkg-config google_cloud_cpp_grpc_utils --libs-only-l --static
...
real    0m0.226s

devjgm added a commit to devjgm/google-cloud-cpp that referenced this issue Jul 26, 2021
Fixes: googleapis#7052

This avoids Fedora's `pkgconf` (a drop-in replacement for `pkg-config`)
because it's too slow when dealing with `.pc` files with lots of
`Requires:` deps, which we have with Abseil. Instead, we use the normal
`pkg-config` binary, which seems to work better.

After this PR, we should be able to upgrade to gRPC 1.39.0 (`git revert
ba41d5a`)
devjgm added a commit that referenced this issue Jul 26, 2021
Fixes: #7052

This avoids Fedora's `pkgconf` (a drop-in replacement for `pkg-config`)
because it's too slow when dealing with `.pc` files with lots of
`Requires:` deps, which we have with Abseil. Instead, we use the normal
`pkg-config` binary, which seems to work better.

After this PR, we should be able to upgrade to gRPC 1.39.0 (`git revert
ba41d5a`)
@veblush
Copy link

veblush commented Jul 28, 2021

@devjgm Thank you for the investigation! I didn't know there is another pkg-config, pkgconf. From what Fedora said, chances are more Linux distributions would replace pkg-config with pkgconf so fixing this problem will save us in future. In that sense, thank you for filing the issue on pkgconf!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants