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

compressor: Allow to optionally use zlib-ng #12408

Merged
merged 50 commits into from
Aug 26, 2020
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
a6baa68
build: Add option to use zlib-ng as compression provider
dio Aug 1, 2020
eb6adb1
Test if zlib=ng works on CI
dio Aug 1, 2020
d73d356
ZLIB_COMPAT
dio Aug 1, 2020
d263984
Skip windows for now
dio Aug 1, 2020
6dc37c9
WITH_OPTIM off
dio Aug 1, 2020
7f55c63
Update doc
dio Aug 1, 2020
9c8a4ec
Strict compat, no optimization, no new strategies
dio Aug 1, 2020
4c89236
More comments
dio Aug 1, 2020
a92cc2b
Fix format
dio Aug 1, 2020
e7dae79
Limit to clang and fix asan
dio Aug 2, 2020
2c1cbdb
@bazel_tools//tools/cpp:clang constraint_values
dio Aug 2, 2020
d656fd5
zlib_ng_with_optimization too
dio Aug 2, 2020
6f41044
Fix gcc
dio Aug 2, 2020
6b490e6
zlib=ng
dio Aug 2, 2020
804dcff
Not sure if this is the fix
dio Aug 2, 2020
f172baf
Only for Linux:x86_64 for now
dio Aug 2, 2020
ce49e45
Checking on ARM64
dio Aug 2, 2020
49aeb5b
Remove turning on zlib-ng
dio Aug 3, 2020
101d208
Merge remote-tracking branch 'upstream/master'
dio Aug 4, 2020
cb13790
Review comments
dio Aug 4, 2020
c16ab8b
Merge remote-tracking branch 'upstream/master'
dio Aug 4, 2020
865b1ad
Update to tip and add 's'
dio Aug 5, 2020
97d36dc
Test handpicked zlib_ng_with_optimizations options
dio Aug 5, 2020
f2e42f4
Remove .
dio Aug 5, 2020
874c077
Make zlib=ng-with-optimizations optional
dio Aug 5, 2020
52a48cb
Merge remote-tracking branch
dio Aug 5, 2020
b565b01
Add compression/libraries section
dio Aug 13, 2020
34796a7
Merge remote-tracking branch 'upstream/master' into zlib-ng
dio Aug 13, 2020
12f6b3c
Merge remote-tracking branch 'upstream/master'
dio Aug 15, 2020
8b1c002
Add zlib=ng-with-optimizations to COMPILE_TIME_OPTIONS
dio Aug 19, 2020
9bb39da
Add comments
dio Aug 19, 2020
eddc6d8
Put at the end
dio Aug 19, 2020
44e4b2b
Fix comment
dio Aug 19, 2020
2c324d6
Fix
dio Aug 19, 2020
684571f
Bring back
dio Aug 19, 2020
bbf24eb
Merge remote-tracking branch 'upstream/master'
dio Aug 19, 2020
bbde62c
Add project metadata
dio Aug 19, 2020
87fe5ea
Missing s
dio Aug 19, 2020
04c930d
Use snake case
dio Aug 22, 2020
3cfd480
Remove zlib=ng_with_optimizations
dio Aug 24, 2020
500e6d5
Merge remote-tracking branch 'upstream/master'
dio Aug 24, 2020
25edeeb
Merge remote-tracking branch 'upstream/master'
dio Aug 25, 2020
008aec2
Attempt
dio Aug 26, 2020
046fd0d
Merge cache entries
dio Aug 26, 2020
0161406
Remove top level alias
dio Aug 26, 2020
71cbe99
Minimize the diff
dio Aug 26, 2020
cc9ab53
Trim
dio Aug 26, 2020
bf8ece6
grpc
dio Aug 26, 2020
ff08725
Fix
dio Aug 26, 2020
8a364a1
Fix format
dio Aug 26, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 38 additions & 0 deletions bazel/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,22 @@ config_setting(
values = {"define": "boringssl=disabled"},
)

config_setting(
name = "zlib_ng",
mattklein123 marked this conversation as resolved.
Show resolved Hide resolved
constraint_values = [
"@bazel_tools//platforms:linux",
],
values = {"define": "zlib=ng"},
)

config_setting(
name = "zlib_ng_with_optimizations",
constraint_values = [
"@bazel_tools//platforms:linux",
],
values = {"define": "zlib=ng-with-optimizations"},
)

config_setting(
name = "enable_quiche",
values = {"define": "quiche=enabled"},
Expand All @@ -259,6 +275,28 @@ alias(
}),
)

# Alias pointing to the selected version of zlib:
# - zlib-ng from //bazel/foreign_cc:zlib_ng,
# - zlib-ng with optimization from //bazel/foreign_cc:zlib_ng_with_optimizations,
# - zlib from //bazel/foreign_cc:zlib.
alias(
name = "zlib",
actual = select({
"//bazel:zlib_ng": "//bazel/foreign_cc:zlib_ng",
"//bazel:zlib_ng_with_optimizations": "//bazel/foreign_cc:zlib_ng_with_optimizations",
"//conditions:default": "//bazel/foreign_cc:zlib",
}),
)

alias(
name = "curl",
actual = select({
"//bazel:zlib_ng": "//bazel/foreign_cc:curl_with_zlib_ng",
"//bazel:zlib_ng_with_optimizations": "//bazel/foreign_cc:curl_with_zlib_ng_with_optimizations",
"//conditions:default": "//bazel/foreign_cc:curl",
}),
)

config_setting(
name = "linux_x86_64",
values = {"cpu": "k8"},
Expand Down
29 changes: 29 additions & 0 deletions bazel/envoy_build_system.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ def envoy_cmake_external(
pdb_name = "",
cmake_files_dir = "$BUILD_TMPDIR/CMakeFiles",
generate_crosstool_file = False,
variants = [],
dio marked this conversation as resolved.
Show resolved Hide resolved
**kwargs):
cache_entries.update({"CMAKE_BUILD_TYPE": "Bazel"})
cache_entries_debug = dict(cache_entries)
Expand Down Expand Up @@ -120,6 +121,34 @@ def envoy_cmake_external(
**kwargs
)

for variant in variants:
if "cache_entries" in variant:
cache_entries.update(variant["cache_entries"] or {})
cache_entries_debug = dict(cache_entries)
cache_entries_debug.update(debug_cache_entries)

if "lib_source" in variant:
lib_source = variant["lib_source"] or lib_source

cmake_external(
name = variant["name"],
cache_entries = select({
"@envoy//bazel:opt_build": cache_entries,
"//conditions:default": cache_entries_debug,
}),
cmake_options = cmake_options,
# TODO(lizan): Make this always true
generate_crosstool_file = select({
"@envoy//bazel:windows_x86_64": True,
"//conditions:default": generate_crosstool_file,
}),
lib_source = lib_source,
make_commands = make_commands,
postfix_script = pf,
static_libraries = static_libraries,
**kwargs
)

# Used to select a dependency that has different implementations on POSIX vs Windows.
# The platform-specific implementations should be specified with envoy_cc_posix_library
# and envoy_cc_win32_library respectively
Expand Down
60 changes: 59 additions & 1 deletion bazel/foreign_cc/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,27 @@ envoy_cmake_external(
"//bazel:windows_x86_64": ["libcurl.lib"],
"//conditions:default": ["libcurl.a"],
}),
variants = [
{
"name": "curl_with_zlib_ng",
"cache_entries": {
"ZLIB_LIBRARY": "$EXT_BUILD_DEPS/zlib_ng",
"ZLIB_INCLUDE_DIR": "$EXT_BUILD_DEPS/zlib_ng/include",
},
},
{
"name": "curl_with_zlib_ng_with_optimizations",
"cache_entries": {
"ZLIB_LIBRARY": "$EXT_BUILD_DEPS/zlib_ng_with_optimizations",
"ZLIB_INCLUDE_DIR": "$EXT_BUILD_DEPS/zlib_ng_with_optimizations/include",
},
},
],
deps = [
":ares",
":nghttp2",
":zlib",
"//external:ssl",
"//external:zlib",
],
)

Expand Down Expand Up @@ -224,4 +240,46 @@ envoy_cmake_external(
"//bazel:windows_x86_64": ["zlibstatic.lib"],
"//conditions:default": ["libz.a"],
}),
variants = [
{
"name": "zlib_ng",
"cache_entries": {
# zlib-ng build options. Reference: https://github.com/zlib-ng/zlib-ng#build-options.
"ZLIB_COMPAT": "on",
"ZLIB_ENABLE_TESTS": "off",

# Turning off WITH_OPTIM and WITH_NEW_STRATEGIES fixes checksum, but not sure if that is
# what we want.
"WITH_OPTIM": "off",
"WITH_NEW_STRATEGIES": "off",

# zlib-ng advance build options. Reference: https://github.com/zlib-ng/zlib-ng#advanced-build-options.
# Only allow aligned address.
"UNALIGNED_OK": "off",
},
"lib_source": "@com_github_zlib_ng_zlib_ng//:all",
},
{
"name": "zlib_ng_with_optimizations",
"cache_entries": {
# zlib-ng build options. Reference: https://github.com/zlib-ng/zlib-ng#build-options.
"ZLIB_COMPAT": "on",
"ZLIB_ENABLE_TESTS": "off",

# Warning: Turning WITH_OPTIM to "on" doesn't pass ZlibCompressorImplTest.CallingChecksum.
"WITH_OPTIM": "on",
# However turning off SSE4 fixes it.
"WITH_SSE4": "off",

# Warning: Turning WITH_NEW_STRATEGIES to "on" doesn't pass gzip compressor fuzz test.
# Turning this off means falling into NO_QUICK_STRATEGY route.
"WITH_NEW_STRATEGIES": "off",

# zlib-ng advance build options. Reference: https://github.com/zlib-ng/zlib-ng#advanced-build-options.
# Only allow aligned address.
"UNALIGNED_OK": "off",
Copy link
Member Author

@dio dio Aug 5, 2020

Choose a reason for hiding this comment

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

I tweaked the build options to be this way to make sure we can get through the required tests on CI (pre-submit CI run: https://github.com/envoyproxy/envoy/runs/947549350) for --define ng-with-optimizations. Feel free to tweak it further. cc. @rgs1

Copy link
Member

Choose a reason for hiding this comment

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

LGTM -- thanks!

},
"lib_source": "@com_github_zlib_ng_zlib_ng//:all",
},
],
)
35 changes: 23 additions & 12 deletions bazel/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,23 @@ def envoy_dependencies(skip_targets = []):
actual = "@envoy//bazel:boringssl",
)

# Binding to an alias pointing to the selected version of zlib:
# - zlib-ng from @envoy//bazel/foreign_cc:zlib_ng,
# - zlib-ng with optimization from @envoy//bazel/foreign_cc:zlib_ng_with_optimizations,
# - zlib from @envoy//bazel/foreign_cc:zlib.
_net_zlib()
_com_github_zlib_ng_zlib_ng()
native.bind(
name = "zlib",
actual = "@envoy//bazel:zlib",
)

# Bind for gRPC.
native.bind(
name = "madler_zlib",
actual = "@envoy//bazel:zlib",
)

# The long repo names (`com_github_fmtlib_fmt` instead of `fmtlib`) are
# semi-standard in the Bazel community, intended to avoid both duplicate
# dependencies and name conflicts.
Expand Down Expand Up @@ -189,7 +206,6 @@ def envoy_dependencies(skip_targets = []):
_com_googlesource_googleurl()
_com_lightstep_tracer_cpp()
_io_opentracing_cpp()
_net_zlib()
_upb()
_proxy_wasm_cpp_sdk()
_proxy_wasm_cpp_host()
Expand Down Expand Up @@ -367,15 +383,10 @@ def _net_zlib():
patches = ["@envoy//bazel/foreign_cc:zlib.patch"],
)

native.bind(
name = "zlib",
actual = "@envoy//bazel/foreign_cc:zlib",
)

# Bind for grpc.
native.bind(
name = "madler_zlib",
actual = "@envoy//bazel/foreign_cc:zlib",
def _com_github_zlib_ng_zlib_ng():
_repository_impl(
name = "com_github_zlib_ng_zlib_ng",
build_file_content = BUILD_ALL_CONTENT,
)

def _com_google_cel_cpp():
Expand Down Expand Up @@ -662,15 +673,15 @@ def _com_github_curl():
http_archive(
name = "com_github_curl",
build_file_content = BUILD_ALL_CONTENT + """
cc_library(name = "curl", visibility = ["//visibility:public"], deps = ["@envoy//bazel/foreign_cc:curl"])
cc_library(name = "curl", visibility = ["//visibility:public"], deps = ["@envoy//bazel:curl"])
""",
patches = ["@envoy//bazel/foreign_cc:curl-revert-cmake-minreqver.patch"],
patch_args = ["-p1"],
**location
)
native.bind(
name = "curl",
actual = "@envoy//bazel/foreign_cc:curl",
actual = "@envoy//bazel:curl",
)

def _com_googlesource_chromium_v8():
Expand Down
8 changes: 8 additions & 0 deletions bazel/repository_locations.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,14 @@ DEPENDENCY_REPOSITORIES = dict(
use_category = ["dataplane"],
cpe = "cpe:2.3:a:gnu:zlib:*",
),
com_github_zlib_ng_zlib_ng = dict(
sha256 = "eb1b3a33855e8994ee89901e74ca4317f3bff93d2e0f3fc5a11e655a78a73092",
strip_prefix = "zlib-ng-8832d7db7241194fa68509c96c092f3cf527ccce",
# 2020-08-03 develop branch.
urls = ["https://github.com/zlib-ng/zlib-ng/archive/8832d7db7241194fa68509c96c092f3cf527ccce.tar.gz"],
use_category = ["dataplane"],
cpe = "N/A",
),
com_github_jbeder_yaml_cpp = dict(
sha256 = "79ab7069ef1c7c3632e7ffe095f7185d4c77b64d8035db3c085c239d4fe96d5f",
strip_prefix = "yaml-cpp-98acc5a8874faab28b82c28936f4b400b389f5d6",
Expand Down
1 change: 1 addition & 0 deletions docs/root/extending/extending.rst
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ types including:
* BoringSSL private key methods
* :ref:`Watchdog action <envoy_v3_api_msg_config.bootstrap.v3.Watchdog.WatchdogAction>`
* :ref:`Internal redirect policy <envoy_v3_api_field_config.route.v3.InternalRedirectPolicy.predicates>`
* :ref:`Compression libraries <arch_overview_compression_libraries>`

As of this writing there is no high level extension developer documentation. The
:repo:`existing extensions <source/extensions>` are a good way to learn what is possible.
Expand Down
1 change: 1 addition & 0 deletions docs/root/intro/arch_overview/arch_overview.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Architecture overview
observability/observability
security/security
operations/operations
compression/libraries
other_features/other_features
other_protocols/other_protocols
advanced/advanced
21 changes: 21 additions & 0 deletions docs/root/intro/arch_overview/compression/libraries.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
.. _arch_overview_compression_libraries:

Compression Libraries
=====================

Underlying implementation
-------------------------

Currently Envoy uses `zlib <http://zlib.net>`_ as a compression library.

.. note::

`zlib-ng <https://github.com/zlib-ng/zlib-ng>`_ is a fork that hosts several 3rd-party
contributions containing new optimizations. Those optimizations are considered useful for
`improving compression performance <https://github.com/envoyproxy/envoy/issues/8448#issuecomment-667152013>`_.
Envoy can be built to use `zlib-ng <https://github.com/zlib-ng/zlib-ng>`_ instead of regular
`zlib <http://zlib.net>`_ by using ``--define zlib=ng`` Bazel option. In order to use
`zlib-ng <https://github.com/zlib-ng/zlib-ng>`_ with optimizations turned on, apply ``--define
zlib=ng-with-optimization``. Please note that building Envoy with ``ng-with-optimization`` means
having a different behavior to generate checksums. Currently, these options are only
available on Linux.
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ New Features
* watchdog: support randomizing the watchdog's kill timeout to prevent synchronized kills via a maximium jitter parameter :ref:`max_kill_timeout_jitter<envoy_v3_api_field_config.bootstrap.v3.Watchdog.max_kill_timeout_jitter>`.
* watchdog: supports an extension point where actions can be registered to fire on watchdog events such as miss, megamiss, kill and multikill. See ref:`watchdog actions<envoy_v3_api_field_config.bootstrap.v3.Watchdog.actions>`.
* xds: added :ref:`extension config discovery<envoy_v3_api_msg_config.core.v3.ExtensionConfigSource>` support for HTTP filters.
* zlib: added option to use `zlib-ng <https://github.com/zlib-ng/zlib-ng>`_ as zlib library.

Deprecated
----------
Expand Down