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 19 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_optimization",
constraint_values = [
"@bazel_tools//platforms:linux",
],
values = {"define": "zlib=ng-with-optimization"},
)

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_optimization,
# - zlib from //bazel/foreign_cc:zlib.
alias(
name = "zlib",
actual = select({
"//bazel:zlib_ng": "//bazel/foreign_cc:zlib_ng",
"//bazel:zlib_ng_with_optimization": "//bazel/foreign_cc:zlib_ng_with_optimization",
"//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_optimization": "//bazel/foreign_cc:curl_with_zlib_ng_with_optimization",
"//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
56 changes: 55 additions & 1 deletion bazel/foreign_cc/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,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_optimization",
"cache_entries": {
"ZLIB_LIBRARY": "$EXT_BUILD_DEPS/zlib_ng_with_optimization",
"ZLIB_INCLUDE_DIR": "$EXT_BUILD_DEPS/zlib_ng_with_optimization/include",
},
},
],
deps = [
":ares",
":nghttp2",
":zlib",
"//external:ssl",
"//external:zlib",
],
)

Expand Down Expand Up @@ -237,4 +253,42 @@ 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_optimization",
"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",
# Warning: Turning WITH_NEW_STRATEGIES to "on" doesn't pass gzip compressor fuzz test.
"WITH_NEW_STRATEGIES": "on",

# zlib-ng advance build options. Reference: https://github.com/zlib-ng/zlib-ng#advanced-build-options.
# Since UNALIGNED_OK is on by default, it is assumed to be one of the optimizations.
"UNALIGNED_OK": "on",
},
"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_optimization,
# - 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 @@ -370,15 +386,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 @@ -665,15 +676,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 @@ -259,6 +259,14 @@ DEPENDENCY_REPOSITORIES = dict(
use_category = ["dataplane"],
cpe = "cpe:2.3:a:gnu:zlib:*",
),
com_github_zlib_ng_zlib_ng = dict(
sha256 = "9646501422faf7a0b3a1d82402a421c0d56c9f4e1926692b6ed0bd80042da209",
strip_prefix = "zlib-ng-c71ca170224f31a219513aa470f82aa50a3ded48",
# 2020-07-31 develop branch.
dio marked this conversation as resolved.
Show resolved Hide resolved
urls = ["https://github.com/zlib-ng/zlib-ng/archive/c71ca170224f31a219513aa470f82aa50a3ded48.tar.gz"],
use_category = ["dataplane"],
cpe = "N/A",
),
com_github_jbeder_yaml_cpp = dict(
sha256 = "79ab7069ef1c7c3632e7ffe095f7185d4c77b64d8035db3c085c239d4fe96d5f",
strip_prefix = "yaml-cpp-98acc5a8874faab28b82c28936f4b400b389f5d6",
Expand Down
20 changes: 20 additions & 0 deletions docs/root/intro/arch_overview/other_features/compression.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
.. _arch_overview_compression:
Copy link
Member

Choose a reason for hiding this comment

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

can we actually this compression page under https://www.envoyproxy.io/docs/envoy/latest/extending/extending. I feel that after all the work that (mostly) @rojkov and I made to make compression a generic construct we should have it there.

Also that way we can say that zlib is a compressor provider rather than the compression provider. wdyt?

Copy link
Member Author

@dio dio Aug 12, 2020

Choose a reason for hiding this comment

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

Sure. I actually wasn’t sure on how to structure the docs. Here I mirrored what we have with boringssl: fips and non-fips. Since zlib is also used by Google gRPC and (for now before it is gone) curl.

Will move it as suggested.

Copy link
Member Author

Choose a reason for hiding this comment

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

@junr03, sorry, a bit confused about how to name/title the section, should it be "compression providers"?

Copy link
Member

Choose a reason for hiding this comment

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

@rojkov and I have used the term "compression libraries" most frequently, including the field name in the compressor/decompressor filters


Compression
===========

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

Currently Envoy is written to use `zlib <http://zlib.net>`_ as the compression provider.
dio marked this conversation as resolved.
Show resolved Hide resolved

.. 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 with `zlib-ng <https://github.com/zlib-ng/zlib-ng>`_ support by using
dio marked this conversation as resolved.
Show resolved Hide resolved
``--define zlib=ng`` Bazel option. In order to use `zlib-ng <https://github.com/zlib-ng/zlib-ng>`_
with optimization turned on, apply ``--define zlib=ng-with-optimization``. Please note that
dio marked this conversation as resolved.
Show resolved Hide resolved
building Envoy with ``ng-with-optimization`` means to have a different behavior on how it
dio marked this conversation as resolved.
Show resolved Hide resolved
accomplishes checksum. Currently, these options are only available on Linux.
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ Other features
global_rate_limiting
scripting
ip_transparency
compression
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ New Features
* tcp: switched the TCP connection pool to the new "shared" connection pool, sharing a common code base with HTTP and HTTP/2. Any unexpected behavioral changes can be temporarily reverted by setting `envoy.reloadable_features.new_tcp_connection_pool` to false.
* 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>`.
* 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