Merge pull request #16777 from jtattermusch/csharp_optimized_grpc_csharp_ext

build assembly-optimized grpc_csharp_ext with cmake

17 files changed, 119 insertions, 32 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index dcd7adeebe..66799c456f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -40,6 +40,7 @@ set(gRPC_INSTALL_SHAREDIR "share/grpc" CACHE STRING "Installation directory for
 option(gRPC_BUILD_TESTS "Build tests" OFF)
 option(gRPC_BUILD_CODEGEN "Build codegen" ON)
 option(gRPC_BUILD_CSHARP_EXT "Build C# extensions" ON)
+option(gRPC_BACKWARDS_COMPATIBILITY_MODE "Build libraries that are binary compatible across a larger number of OS and libc versions" OFF)
 
 set(gRPC_INSTALL_default ON)
 if (NOT CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
@@ -115,6 +116,14 @@ else()
   set(_gRPC_PROTOBUF_LIBRARY_NAME "libprotobuf")
 endif()
 
+if(gRPC_BACKWARDS_COMPATIBILITY_MODE)
+  add_definitions(-DGPR_BACKWARDS_COMPATIBILITY_MODE)
+  if (_gRPC_PLATFORM_MAC)
+    # some C++11 constructs not supported before OS X 10.9
+    set(CMAKE_OSX_DEPLOYMENT_TARGET 10.9)
+  endif()
+endif()
+
 if (_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC)
   # C core has C++ source code, but should not depend on libstc++ (for better portability).
   # We need to use a few tricks to convince cmake to do that.
diff --git a/doc/ssl-performance.md b/doc/ssl-performance.md
index 711b9dff09..3de1ecac00 100644
--- a/doc/ssl-performance.md
+++ b/doc/ssl-performance.md
@@ -25,7 +25,10 @@ In addition, we are shipping packages for language implementations. These packag
 
 Language | From source | Platform | Uses assembly optimizations
 ---|---|---|---
-C#      | n/a | all | :x:
+C#      | n/a | Linux, 64bit | :heavy_check_mark:
+C#      | n/a | Linux, 32bit | :x:
+C#      | n/a | MacOS | :heavy_check_mark:
+C#      | n/a | Windows | :x:
 Node.JS | n/a | Linux | :heavy_check_mark:
 Node.JS | n/a | MacOS | :heavy_check_mark:
 Node.JS | n/a | Windows | :x:
diff --git a/src/csharp/Grpc.Core/NativeDeps.Linux.csproj.include b/src/csharp/Grpc.Core/NativeDeps.Linux.csproj.include
index e3bbeb071e..af660064a4 100644
--- a/src/csharp/Grpc.Core/NativeDeps.Linux.csproj.include
+++ b/src/csharp/Grpc.Core/NativeDeps.Linux.csproj.include
@@ -1,6 +1,6 @@
 <Project>
   <ItemGroup>
-    <Content Include="..\..\..\libs\$(NativeDependenciesConfigurationUnix)\libgrpc_csharp_ext.so">
+    <Content Include="..\..\..\cmake\build\libgrpc_csharp_ext.so">
       <Link>libgrpc_csharp_ext.x64.so</Link>
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
       <Pack>false</Pack>
diff --git a/src/csharp/Grpc.Core/NativeDeps.Mac.csproj.include b/src/csharp/Grpc.Core/NativeDeps.Mac.csproj.include
index 309e33d47e..570b0cd8b7 100644
--- a/src/csharp/Grpc.Core/NativeDeps.Mac.csproj.include
+++ b/src/csharp/Grpc.Core/NativeDeps.Mac.csproj.include
@@ -1,6 +1,6 @@
 <Project>
   <ItemGroup>
-    <Content Include="..\..\..\libs\$(NativeDependenciesConfigurationUnix)\libgrpc_csharp_ext.dylib">
+    <Content Include="..\..\..\cmake\build\libgrpc_csharp_ext.dylib">
       <Link>libgrpc_csharp_ext.x64.dylib</Link>
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
       <Pack>false</Pack>
diff --git a/templates/CMakeLists.txt.template b/templates/CMakeLists.txt.template
index aec1624acd..1628493d00 100644
--- a/templates/CMakeLists.txt.template
+++ b/templates/CMakeLists.txt.template
@@ -88,6 +88,7 @@
   option(gRPC_BUILD_TESTS "Build tests" OFF)
   option(gRPC_BUILD_CODEGEN "Build codegen" ON)
   option(gRPC_BUILD_CSHARP_EXT "Build C# extensions" ON)
+  option(gRPC_BACKWARDS_COMPATIBILITY_MODE "Build libraries that are binary compatible across a larger number of OS and libc versions" OFF)
 
   set(gRPC_INSTALL_default ON)
   if (NOT CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
@@ -164,6 +165,14 @@
     set(_gRPC_PROTOBUF_LIBRARY_NAME "libprotobuf")
   endif()
 
+  if(gRPC_BACKWARDS_COMPATIBILITY_MODE)
+    add_definitions(-DGPR_BACKWARDS_COMPATIBILITY_MODE)
+    if (_gRPC_PLATFORM_MAC)
+      # some C++11 constructs not supported before OS X 10.9
+      set(CMAKE_OSX_DEPLOYMENT_TARGET 10.9)
+    endif()
+  endif()
+
   if (_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC)
     # C core has C++ source code, but should not depend on libstc++ (for better portability).
     # We need to use a few tricks to convince cmake to do that.
diff --git a/templates/tools/dockerfile/csharp_deps.include b/templates/tools/dockerfile/csharp_deps.include
index 3a40711e37..7ed0074867 100644
--- a/templates/tools/dockerfile/csharp_deps.include
+++ b/templates/tools/dockerfile/csharp_deps.include
@@ -15,3 +15,10 @@ RUN apt-get update && apt-get -y dist-upgrade && apt-get install -y ${'\\'}
     && apt-get clean
 
 RUN nuget update -self
+
+#=================
+# Use cmake 3.6 from jessie-backports
+# needed to build grpc_csharp_ext with cmake
+
+RUN echo "deb http://ftp.debian.org/debian jessie-backports main" | tee /etc/apt/sources.list.d/jessie-backports.list
+RUN apt-get update && apt-get install -t jessie-backports -y cmake && apt-get clean
diff --git a/tools/dockerfile/grpc_artifact_linux_x64/Dockerfile b/tools/dockerfile/grpc_artifact_linux_x64/Dockerfile
index 228efef698..7ec061ebe5 100644
--- a/tools/dockerfile/grpc_artifact_linux_x64/Dockerfile
+++ b/tools/dockerfile/grpc_artifact_linux_x64/Dockerfile
@@ -68,6 +68,13 @@ RUN apt-get update && apt-get install -y \
     php5 php5-dev php-pear phpunit && apt-get clean
 
 
+##################
+# C# dependencies (needed to build grpc_csharp_ext)
+
+# Use cmake 3.6 from jessie-backports
+RUN echo "deb http://ftp.debian.org/debian jessie-backports main" | tee /etc/apt/sources.list.d/jessie-backports.list
+RUN apt-get update && apt-get install -t jessie-backports -y cmake && apt-get clean
+
 RUN mkdir /var/local/jenkins
 
 # Define the default command.
diff --git a/tools/dockerfile/grpc_artifact_linux_x86/Dockerfile b/tools/dockerfile/grpc_artifact_linux_x86/Dockerfile
index d33e0f83ea..f81d8e5ba0 100644
--- a/tools/dockerfile/grpc_artifact_linux_x86/Dockerfile
+++ b/tools/dockerfile/grpc_artifact_linux_x86/Dockerfile
@@ -60,6 +60,13 @@ RUN /bin/bash -l -c "echo 'export PATH=/usr/local/rvm/bin:$PATH' >> ~/.bashrc"
 RUN /bin/bash -l -c "echo 'rvm --default use ruby-2.1' >> ~/.bashrc"
 RUN /bin/bash -l -c "gem install bundler --no-ri --no-rdoc"
 
+##################
+# C# dependencies (needed to build grpc_csharp_ext)
+
+# Use cmake 3.6 from jessie-backports
+RUN echo "deb http://ftp.debian.org/debian jessie-backports main" | tee /etc/apt/sources.list.d/jessie-backports.list
+RUN apt-get update && apt-get install -t jessie-backports -y cmake && apt-get clean
+
 RUN mkdir /var/local/jenkins
 
 # Define the default command.
diff --git a/tools/dockerfile/interoptest/grpc_interop_csharp/Dockerfile b/tools/dockerfile/interoptest/grpc_interop_csharp/Dockerfile
index 511e2932d6..b2216c79d4 100644
--- a/tools/dockerfile/interoptest/grpc_interop_csharp/Dockerfile
+++ b/tools/dockerfile/interoptest/grpc_interop_csharp/Dockerfile
@@ -82,6 +82,13 @@ RUN apt-get update && apt-get -y dist-upgrade && apt-get install -y \
 
 RUN nuget update -self
 
+#=================
+# Use cmake 3.6 from jessie-backports
+# needed to build grpc_csharp_ext with cmake
+
+RUN echo "deb http://ftp.debian.org/debian jessie-backports main" | tee /etc/apt/sources.list.d/jessie-backports.list
+RUN apt-get update && apt-get install -t jessie-backports -y cmake && apt-get clean
+
 # Install dotnet SDK based on https://www.microsoft.com/net/core#debian
 RUN apt-get update && apt-get install -y curl libunwind8 gettext
 # dotnet-dev-1.0.0-preview2-003131
diff --git a/tools/dockerfile/interoptest/grpc_interop_csharpcoreclr/Dockerfile b/tools/dockerfile/interoptest/grpc_interop_csharpcoreclr/Dockerfile
index 511e2932d6..b2216c79d4 100644
--- a/tools/dockerfile/interoptest/grpc_interop_csharpcoreclr/Dockerfile
+++ b/tools/dockerfile/interoptest/grpc_interop_csharpcoreclr/Dockerfile
@@ -82,6 +82,13 @@ RUN apt-get update && apt-get -y dist-upgrade && apt-get install -y \
 
 RUN nuget update -self
 
+#=================
+# Use cmake 3.6 from jessie-backports
+# needed to build grpc_csharp_ext with cmake
+
+RUN echo "deb http://ftp.debian.org/debian jessie-backports main" | tee /etc/apt/sources.list.d/jessie-backports.list
+RUN apt-get update && apt-get install -t jessie-backports -y cmake && apt-get clean
+
 # Install dotnet SDK based on https://www.microsoft.com/net/core#debian
 RUN apt-get update && apt-get install -y curl libunwind8 gettext
 # dotnet-dev-1.0.0-preview2-003131
diff --git a/tools/dockerfile/test/csharp_jessie_x64/Dockerfile b/tools/dockerfile/test/csharp_jessie_x64/Dockerfile
index 56bfb89925..578bf427cd 100644
--- a/tools/dockerfile/test/csharp_jessie_x64/Dockerfile
+++ b/tools/dockerfile/test/csharp_jessie_x64/Dockerfile
@@ -86,6 +86,13 @@ RUN apt-get update && apt-get -y dist-upgrade && apt-get install -y \
 
 RUN nuget update -self
 
+#=================
+# Use cmake 3.6 from jessie-backports
+# needed to build grpc_csharp_ext with cmake
+
+RUN echo "deb http://ftp.debian.org/debian jessie-backports main" | tee /etc/apt/sources.list.d/jessie-backports.list
+RUN apt-get update && apt-get install -t jessie-backports -y cmake && apt-get clean
+
 # Install dotnet SDK based on https://www.microsoft.com/net/core#debian
 RUN apt-get update && apt-get install -y curl libunwind8 gettext
 # dotnet-dev-1.0.0-preview2-003131
diff --git a/tools/dockerfile/test/multilang_jessie_x64/Dockerfile b/tools/dockerfile/test/multilang_jessie_x64/Dockerfile
index a82e7050fc..ad719f330f 100644
--- a/tools/dockerfile/test/multilang_jessie_x64/Dockerfile
+++ b/tools/dockerfile/test/multilang_jessie_x64/Dockerfile
@@ -71,6 +71,13 @@ RUN apt-get update && apt-get -y dist-upgrade && apt-get install -y \
 
 RUN nuget update -self
 
+#=================
+# Use cmake 3.6 from jessie-backports
+# needed to build grpc_csharp_ext with cmake
+
+RUN echo "deb http://ftp.debian.org/debian jessie-backports main" | tee /etc/apt/sources.list.d/jessie-backports.list
+RUN apt-get update && apt-get install -t jessie-backports -y cmake && apt-get clean
+
 # Install dotnet SDK based on https://www.microsoft.com/net/core#debian
 RUN apt-get update && apt-get install -y curl libunwind8 gettext
 # dotnet-dev-1.0.0-preview2-003131
diff --git a/tools/run_tests/artifacts/artifact_targets.py b/tools/run_tests/artifacts/artifact_targets.py
index bdeb258e1f..054561aac9 100644
--- a/tools/run_tests/artifacts/artifact_targets.py
+++ b/tools/run_tests/artifacts/artifact_targets.py
@@ -248,29 +248,31 @@ class CSharpExtArtifact:
                 ],
                 use_workspace=True)
         else:
-            environ = {
-                'CONFIG': 'opt',
-                'EMBED_OPENSSL': 'true',
-                'EMBED_ZLIB': 'true',
-                'CFLAGS': '-DGPR_BACKWARDS_COMPATIBILITY_MODE',
-                'CXXFLAGS': '-DGPR_BACKWARDS_COMPATIBILITY_MODE',
-                'LDFLAGS': ''
-            }
             if self.platform == 'linux':
+                cmake_arch_option = ''  # x64 is the default architecture
+                if self.arch == 'x86':
+                    # TODO(jtattermusch): more work needed to enable
+                    # boringssl assembly optimizations for 32-bit linux.
+                    # Problem: currently we are building the artifact under
+                    # 32-bit docker image, but CMAKE_SYSTEM_PROCESSOR is still
+                    # set to x86_64, so the resulting boringssl binary
+                    # would have undefined symbols.
+                    cmake_arch_option = '-DOPENSSL_NO_ASM=ON'
                 return create_docker_jobspec(
                     self.name,
                     'tools/dockerfile/grpc_artifact_linux_%s' % self.arch,
                     'tools/run_tests/artifacts/build_artifact_csharp.sh',
-                    environ=environ)
+                    environ={
+                        'CMAKE_ARCH_OPTION': cmake_arch_option
+                    })
             else:
-                archflag = _ARCH_FLAG_MAP[self.arch]
-                environ['CFLAGS'] += ' %s %s' % (archflag, _MACOS_COMPAT_FLAG)
-                environ['CXXFLAGS'] += ' %s %s' % (archflag, _MACOS_COMPAT_FLAG)
-                environ['LDFLAGS'] += ' %s' % archflag
+                cmake_arch_option = ''  # x64 is the default architecture
+                if self.arch == 'x86':
+                    cmake_arch_option = '-DCMAKE_OSX_ARCHITECTURES=i386'
                 return create_jobspec(
                     self.name,
                     ['tools/run_tests/artifacts/build_artifact_csharp.sh'],
-                    environ=environ,
+                    environ={'CMAKE_ARCH_OPTION': cmake_arch_option},
                     use_workspace=True)
 
     def __str__(self):
diff --git a/tools/run_tests/artifacts/build_artifact_csharp.sh b/tools/run_tests/artifacts/build_artifact_csharp.sh
index d65340261d..bb8a91b520 100755
--- a/tools/run_tests/artifacts/build_artifact_csharp.sh
+++ b/tools/run_tests/artifacts/build_artifact_csharp.sh
@@ -17,7 +17,17 @@ set -ex
 
 cd "$(dirname "$0")/../../.."
 
-make grpc_csharp_ext
+mkdir -p cmake/build
+cd cmake/build
+
+cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo \
+      -DgRPC_BACKWARDS_COMPATIBILITY_MODE=ON \
+      -DgRPC_BUILD_TESTS=OFF \
+      "${CMAKE_ARCH_OPTION}" \
+      ../..
+
+make grpc_csharp_ext -j2
+cd ../..
 
 mkdir -p "${ARTIFACTS_OUT}"
-cp libs/opt/libgrpc_csharp_ext.so "${ARTIFACTS_OUT}" || cp libs/opt/libgrpc_csharp_ext.dylib "${ARTIFACTS_OUT}"
+cp cmake/build/libgrpc_csharp_ext.so "${ARTIFACTS_OUT}" || cp cmake/build/libgrpc_csharp_ext.dylib "${ARTIFACTS_OUT}"
diff --git a/tools/run_tests/helper_scripts/pre_build_csharp.sh b/tools/run_tests/helper_scripts/pre_build_csharp.sh
index f9f5440a61..7d83986f90 100755
--- a/tools/run_tests/helper_scripts/pre_build_csharp.sh
+++ b/tools/run_tests/helper_scripts/pre_build_csharp.sh
@@ -15,7 +15,14 @@
 
 set -ex
 
-# cd to gRPC csharp directory
-cd "$(dirname "$0")/../../../src/csharp"
+# cd to repository root
+cd "$(dirname "$0")/../../.."
+
+mkdir -p cmake/build
+cd cmake/build
+
+cmake -DgRPC_BUILD_TESTS=OFF -DCMAKE_BUILD_TYPE="${MSBUILD_CONFIG}" ../..
+
+cd ../../src/csharp
 
 dotnet restore Grpc.sln
diff --git a/tools/run_tests/performance/build_performance.sh b/tools/run_tests/performance/build_performance.sh
index 35d9e90598..f235582579 100755
--- a/tools/run_tests/performance/build_performance.sh
+++ b/tools/run_tests/performance/build_performance.sh
@@ -53,7 +53,10 @@ do
     fi
     ;;
   "csharp")
-    python tools/run_tests/run_tests.py -l "$language" -c "$CONFIG" --build_only -j 8 --compiler coreclr
+    python tools/run_tests/run_tests.py -l "$language" -c "$CONFIG" --build_only -j 8
+    # unbreak subsequent make builds by restoring zconf.h (previously renamed by cmake portion of C#'s build)
+    # See https://github.com/grpc/grpc/issues/11581
+    (cd third_party/zlib; git checkout zconf.h)
     ;;
   "node"|"node_purejs")
     tools/run_tests/performance/build_performance_node.sh
diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py
index 3d73f9ec0e..c9b4c8b28b 100755
--- a/tools/run_tests/run_tests.py
+++ b/tools/run_tests/run_tests.py
@@ -922,20 +922,13 @@ class CSharpLanguage(object):
         self.config = config
         self.args = args
         if self.platform == 'windows':
-            _check_compiler(self.args.compiler, ['coreclr', 'default'])
+            _check_compiler(self.args.compiler, ['default', 'coreclr'])
             _check_arch(self.args.arch, ['default'])
             self._cmake_arch_option = 'x64'
-            self._make_options = []
         else:
             _check_compiler(self.args.compiler, ['default', 'coreclr'])
             self._docker_distro = 'jessie'
 
-            if self.platform == 'mac':
-                # TODO(jtattermusch): EMBED_ZLIB=true currently breaks the mac build
-                self._make_options = ['EMBED_OPENSSL=true']
-            else:
-                self._make_options = ['EMBED_OPENSSL=true', 'EMBED_ZLIB=true']
-
     def test_specs(self):
         with open('src/csharp/tests.json') as f:
             tests_by_assembly = json.load(f)
@@ -1010,7 +1003,7 @@ class CSharpLanguage(object):
         return ['grpc_csharp_ext']
 
     def make_options(self):
-        return self._make_options
+        return []
 
     def build_steps(self):
         if self.platform == 'windows':
@@ -1028,7 +1021,9 @@ class CSharpLanguage(object):
         if self.platform == 'windows':
             return 'cmake/build/%s/Makefile' % self._cmake_arch_option
         else:
-            return 'Makefile'
+            # no need to set x86 specific flags as run_tests.py
+            # currently forbids x86 C# builds on both Linux and MacOS.
+            return 'cmake/build/Makefile'
 
     def dockerfile_dir(self):
         return 'tools/dockerfile/test/csharp_%s_%s' % (