Merge branch 'master' into uselogicalthread

.github/ISSUE_TEMPLATE/bug_report.md

@@ -2,7 +2,7 @@
 name: Report a bug
 about: Create a report to help us improve
 labels: kind/bug, priority/P2
-assignees: markdroth 
+assignees: nicolasnoble 
 
 ---
 

.github/ISSUE_TEMPLATE/cleanup_request.md

@@ -2,7 +2,7 @@
 name: Request a cleanup
 about: Suggest a cleanup in our repository
 labels: kind/internal cleanup, priority/P2
-assignees: markdroth 
+assignees: nicolasnoble 
 
 ---
 

.github/ISSUE_TEMPLATE/feature_request.md

@@ -2,7 +2,7 @@
 name: Request a feature
 about: Suggest an idea for this project
 labels: kind/enhancement, priority/P2
-assignees: markdroth 
+assignees: nicolasnoble 
 
 ---
 

.github/pull_request_template.md

@@ -8,4 +8,4 @@ If you know who should review your pull request, please remove the mentioning be
 
 -->
 
-@markdroth
+@nicolasnoble

.gitignore

@@ -115,11 +115,7 @@ Podfile.lock
 .idea/
 
 # Bazel files
-bazel-bin
-bazel-genfiles
-bazel-grpc
-bazel-out
-bazel-testlogs
+bazel-*
 bazel_format_virtual_environment/
 tools/bazel-*
 

.pylintrc

@@ -12,14 +12,14 @@ extension-pkg-whitelist=grpc._cython.cygrpc
 
 # TODO(https://github.com/PyCQA/pylint/issues/1345): How does the inspection
 # not include "unused_" and "ignored_" by default?
-dummy-variables-rgx=^ignored_|^unused_
+dummy-variables-rgx=^ignored_|^unused_|_
 
 [DESIGN]
 
 # NOTE(nathaniel): Not particularly attached to this value; it just seems to
 # be what works for us at the moment (excepting the dead-code-walking Beta
 # API).
-max-args=7
+max-args=14
 max-parents=8
 
 [MISCELLANEOUS]

BUILD

@@ -72,19 +72,14 @@ config_setting(
     values = {"cpu": "darwin"},
 )
 
-config_setting(
-    name = "grpc_disable_absl",
-    values = {"define": "GRPC_USE_ABSL=0"},
-)
-
 python_config_settings()
 
 # This should be updated along with build.yaml
-g_stands_for = "guantao"
+g_stands_for = "galactic"
 
 core_version = "9.0.0"
 
-version = "1.27.0-dev"
+version = "1.28.0-dev"
 
 GPR_PUBLIC_HDRS = [
     "include/grpc/support/alloc.h",
@@ -561,7 +556,9 @@ grpc_cc_library(
         "src/core/lib/profiling/timers.h",
     ],
     external_deps = [
+        "absl/memory",
         "absl/strings",
+        "absl/strings:str_format",
     ],
     language = "c++",
     public_hdrs = GPR_PUBLIC_HDRS,
@@ -746,6 +743,7 @@ grpc_cc_library(
         "src/core/lib/iomgr/iomgr_internal.cc",
         "src/core/lib/iomgr/iomgr_posix.cc",
         "src/core/lib/iomgr/iomgr_posix_cfstream.cc",
+        "src/core/lib/iomgr/iomgr_uv.cc",
         "src/core/lib/iomgr/iomgr_windows.cc",
         "src/core/lib/iomgr/is_epollexclusive_available.cc",
         "src/core/lib/iomgr/load_file.cc",
@@ -769,6 +767,7 @@ grpc_cc_library(
         "src/core/lib/iomgr/socket_utils_common_posix.cc",
         "src/core/lib/iomgr/socket_utils_linux.cc",
         "src/core/lib/iomgr/socket_utils_posix.cc",
+        "src/core/lib/iomgr/socket_utils_uv.cc",
         "src/core/lib/iomgr/socket_utils_windows.cc",
         "src/core/lib/iomgr/socket_windows.cc",
         "src/core/lib/iomgr/tcp_client.cc",
@@ -988,6 +987,7 @@ grpc_cc_library(
     public_hdrs = GRPC_PUBLIC_HDRS,
     use_cfstream = True,
     deps = [
+        "eventmanager_libuv",
         "gpr_base",
         "grpc_codegen",
         "grpc_trace",

BUILD.gn

@@ -162,6 +162,8 @@ config("grpc_config") {
     ]
     deps = [
         ":absl/container:inlined_vector",
+        ":absl/memory:memory",
+        ":absl/strings:str_format",
         ":absl/strings:strings",
         ":absl/types:optional",
     ]

CMakeLists.txt

@@ -25,12 +25,12 @@
 cmake_minimum_required(VERSION 3.5.1)
 
 set(PACKAGE_NAME          "grpc")
-set(PACKAGE_VERSION       "1.27.0-dev")
+set(PACKAGE_VERSION       "1.28.0-dev")
 set(gRPC_CORE_VERSION     "9.0.0")
 set(gRPC_CORE_SOVERSION   "9")
-set(gRPC_CPP_VERSION      "1.27.0-dev")
+set(gRPC_CPP_VERSION      "1.28.0-dev")
 set(gRPC_CPP_SOVERSION    "1")
-set(gRPC_CSHARP_VERSION   "2.27.0-dev")
+set(gRPC_CSHARP_VERSION   "2.28.0-dev")
 set(gRPC_CSHARP_SOVERSION "2")
 set(PACKAGE_STRING        "${PACKAGE_NAME} ${PACKAGE_VERSION}")
 set(PACKAGE_TARNAME       "${PACKAGE_NAME}-${PACKAGE_VERSION}")
@@ -119,6 +119,8 @@ set(gRPC_ABSL_USED_TARGETS
   absl_raw_logging_internal
   absl_span
   absl_spinlock_wait
+  absl_str_format
+  absl_str_format_internal
   absl_strings
   absl_strings_internal
   absl_throw_delegate
@@ -1439,6 +1441,8 @@ target_include_directories(gpr
 target_link_libraries(gpr
   ${_gRPC_ALLTARGETS_LIBRARIES}
   absl::inlined_vector
+  absl::memory
+  absl::str_format
   absl::strings
   absl::optional
 )
@@ -18571,7 +18575,7 @@ generate_pkgconfig(
   "gRPC platform support library"
   "${gRPC_CORE_VERSION}"
   ""
-  "-lgpr -labsl_bad_optional_access -labsl_strings -labsl_strings_internal -labsl_int128 -labsl_base -labsl_spinlock_wait -labsl_dynamic_annotations -labsl_throw_delegate -labsl_raw_logging_internal -labsl_log_severity"
+  "-lgpr -labsl_bad_optional_access -labsl_str_format_internal -labsl_strings -labsl_strings_internal -labsl_base -labsl_spinlock_wait -labsl_dynamic_annotations -labsl_int128 -labsl_throw_delegate -labsl_raw_logging_internal -labsl_log_severity"
   ""
   "gpr.pc")
 
@@ -18581,7 +18585,7 @@ generate_pkgconfig(
   "high performance general RPC framework"
   "${gRPC_CORE_VERSION}"
   "gpr openssl"
-  "-lgrpc -laddress_sorting -lupb -lcares -lz -labsl_bad_optional_access -labsl_strings -labsl_strings_internal -labsl_int128 -labsl_base -labsl_spinlock_wait -labsl_dynamic_annotations -labsl_throw_delegate -labsl_raw_logging_internal -labsl_log_severity"
+  "-lgrpc -laddress_sorting -lupb -lcares -lz -labsl_bad_optional_access -labsl_str_format_internal -labsl_strings -labsl_strings_internal -labsl_base -labsl_spinlock_wait -labsl_dynamic_annotations -labsl_int128 -labsl_throw_delegate -labsl_raw_logging_internal -labsl_log_severity"
   ""
   "grpc.pc")
 
@@ -18591,7 +18595,7 @@ generate_pkgconfig(
   "high performance general RPC framework without SSL"
   "${gRPC_CORE_VERSION}"
   "gpr"
-  "-lgrpc_unsecure -labsl_bad_optional_access -labsl_strings -labsl_strings_internal -labsl_int128 -labsl_base -labsl_spinlock_wait -labsl_dynamic_annotations -labsl_throw_delegate -labsl_raw_logging_internal -labsl_log_severity"
+  "-lgrpc_unsecure -labsl_bad_optional_access -labsl_str_format_internal -labsl_strings -labsl_strings_internal -labsl_base -labsl_spinlock_wait -labsl_dynamic_annotations -labsl_int128 -labsl_throw_delegate -labsl_raw_logging_internal -labsl_log_severity"
   ""
   "grpc_unsecure.pc")
 
@@ -18601,7 +18605,7 @@ generate_pkgconfig(
   "C++ wrapper for gRPC"
   "${PACKAGE_VERSION}"
   "grpc"
-  "-lgrpc++ -labsl_bad_optional_access -labsl_strings -labsl_strings_internal -labsl_int128 -labsl_base -labsl_spinlock_wait -labsl_dynamic_annotations -labsl_throw_delegate -labsl_raw_logging_internal -labsl_log_severity"
+  "-lgrpc++ -labsl_bad_optional_access -labsl_str_format_internal -labsl_strings -labsl_strings_internal -labsl_base -labsl_spinlock_wait -labsl_dynamic_annotations -labsl_int128 -labsl_throw_delegate -labsl_raw_logging_internal -labsl_log_severity"
   ""
   "grpc++.pc")
 
@@ -18611,6 +18615,6 @@ generate_pkgconfig(
   "C++ wrapper for gRPC without SSL"
   "${PACKAGE_VERSION}"
   "grpc_unsecure"
-  "-lgrpc++_unsecure -labsl_bad_optional_access -labsl_strings -labsl_strings_internal -labsl_int128 -labsl_base -labsl_spinlock_wait -labsl_dynamic_annotations -labsl_throw_delegate -labsl_raw_logging_internal -labsl_log_severity"
+  "-lgrpc++_unsecure -labsl_bad_optional_access -labsl_str_format_internal -labsl_strings -labsl_strings_internal -labsl_base -labsl_spinlock_wait -labsl_dynamic_annotations -labsl_int128 -labsl_throw_delegate -labsl_raw_logging_internal -labsl_log_severity"
   ""
   "grpc++_unsecure.pc")

Makefile

@@ -410,7 +410,7 @@ LDFLAGS += -pthread
 endif
 
 ifeq ($(SYSTEM),MINGW32)
-LIBS = m pthread ws2_32
+LIBS = m pthread ws2_32 dbghelp
 LDFLAGS += -pthread
 endif
 
@@ -470,8 +470,8 @@ Q = @
 endif
 
 CORE_VERSION = 9.0.0
-CPP_VERSION = 1.27.0-dev
-CSHARP_VERSION = 2.27.0-dev
+CPP_VERSION = 1.28.0-dev
+CSHARP_VERSION = 2.28.0-dev
 
 CPPFLAGS_NO_ARCH += $(addprefix -I, $(INCLUDES)) $(addprefix -D, $(DEFINES))
 CPPFLAGS += $(CPPFLAGS_NO_ARCH) $(ARCH_FLAGS)
@@ -9089,6 +9089,12 @@ LIBGRPC_ABSEIL_SRC = \
     third_party/abseil-cpp/absl/strings/internal/escaping.cc \
     third_party/abseil-cpp/absl/strings/internal/memutil.cc \
     third_party/abseil-cpp/absl/strings/internal/ostringstream.cc \
+    third_party/abseil-cpp/absl/strings/internal/str_format/arg.cc \
+    third_party/abseil-cpp/absl/strings/internal/str_format/bind.cc \
+    third_party/abseil-cpp/absl/strings/internal/str_format/extension.cc \
+    third_party/abseil-cpp/absl/strings/internal/str_format/float_conversion.cc \
+    third_party/abseil-cpp/absl/strings/internal/str_format/output.cc \
+    third_party/abseil-cpp/absl/strings/internal/str_format/parser.cc \
     third_party/abseil-cpp/absl/strings/internal/utf8.cc \
     third_party/abseil-cpp/absl/strings/match.cc \
     third_party/abseil-cpp/absl/strings/numbers.cc \

README.md

@@ -53,7 +53,7 @@ Sometimes things go wrong. Please check out the [Troubleshooting guide](TROUBLES
 
 # Performance 
 
-See the [Performance dashboard](http://performance-dot-grpc-testing.appspot.com/explore?dashboard=5636470266134528) for performance numbers of the latest released version.
+See the [Performance dashboard](https://performance-dot-grpc-testing.appspot.com/explore?dashboard=5652536396611584) for performance numbers of master branch daily builds.
 
 # Concepts
 

bazel/grpc_build_system.bzl

@@ -100,10 +100,6 @@ def grpc_cc_library(
                       "//:grpc_allow_exceptions": ["GRPC_ALLOW_EXCEPTIONS=1"],
                       "//:grpc_disallow_exceptions": ["GRPC_ALLOW_EXCEPTIONS=0"],
                       "//conditions:default": [],
-                  }) +
-                  select({
-                      "//:grpc_disable_absl": ["GRPC_USE_ABSL=0"],
-                      "//conditions:default": [],
                   }),
         hdrs = hdrs + public_hdrs,
         deps = deps + _get_external_deps(external_deps),

build.yaml

@@ -14,8 +14,8 @@ settings:
   '#10': See the expand_version.py for all the quirks here
   core_version: 9.0.0
   csharp_major_version: 2
-  g_stands_for: guantao
-  version: 1.27.0-dev
+  g_stands_for: galactic
+  version: 1.28.0-dev
 filegroups:
 - name: alts_tsi
   headers:
@@ -271,6 +271,8 @@ filegroups:
   - src/core/lib/profiling/stap_timers.cc
   deps:
   - absl/container:inlined_vector
+  - absl/memory:memory
+  - absl/strings:str_format
   - absl/strings:strings
   - absl/types:optional
   uses:

config.m4

@@ -489,6 +489,12 @@ if test "$PHP_GRPC" != "no"; then
     third_party/abseil-cpp/absl/strings/internal/escaping.cc \
     third_party/abseil-cpp/absl/strings/internal/memutil.cc \
     third_party/abseil-cpp/absl/strings/internal/ostringstream.cc \
+    third_party/abseil-cpp/absl/strings/internal/str_format/arg.cc \
+    third_party/abseil-cpp/absl/strings/internal/str_format/bind.cc \
+    third_party/abseil-cpp/absl/strings/internal/str_format/extension.cc \
+    third_party/abseil-cpp/absl/strings/internal/str_format/float_conversion.cc \
+    third_party/abseil-cpp/absl/strings/internal/str_format/output.cc \
+    third_party/abseil-cpp/absl/strings/internal/str_format/parser.cc \
     third_party/abseil-cpp/absl/strings/internal/utf8.cc \
     third_party/abseil-cpp/absl/strings/match.cc \
     third_party/abseil-cpp/absl/strings/numbers.cc \
@@ -882,6 +888,7 @@ if test "$PHP_GRPC" != "no"; then
   PHP_ADD_BUILD_DIR($ext_builddir/third_party/abseil-cpp/absl/numeric)
   PHP_ADD_BUILD_DIR($ext_builddir/third_party/abseil-cpp/absl/strings)
   PHP_ADD_BUILD_DIR($ext_builddir/third_party/abseil-cpp/absl/strings/internal)
+  PHP_ADD_BUILD_DIR($ext_builddir/third_party/abseil-cpp/absl/strings/internal/str_format)
   PHP_ADD_BUILD_DIR($ext_builddir/third_party/abseil-cpp/absl/types)
   PHP_ADD_BUILD_DIR($ext_builddir/third_party/address_sorting)
   PHP_ADD_BUILD_DIR($ext_builddir/third_party/boringssl-with-bazel)

config.w32

@@ -458,6 +458,12 @@ if (PHP_GRPC != "no") {
     "third_party\\abseil-cpp\\absl\\strings\\internal\\escaping.cc " +
     "third_party\\abseil-cpp\\absl\\strings\\internal\\memutil.cc " +
     "third_party\\abseil-cpp\\absl\\strings\\internal\\ostringstream.cc " +
+    "third_party\\abseil-cpp\\absl\\strings\\internal\\str_format\\arg.cc " +
+    "third_party\\abseil-cpp\\absl\\strings\\internal\\str_format\\bind.cc " +
+    "third_party\\abseil-cpp\\absl\\strings\\internal\\str_format\\extension.cc " +
+    "third_party\\abseil-cpp\\absl\\strings\\internal\\str_format\\float_conversion.cc " +
+    "third_party\\abseil-cpp\\absl\\strings\\internal\\str_format\\output.cc " +
+    "third_party\\abseil-cpp\\absl\\strings\\internal\\str_format\\parser.cc " +
     "third_party\\abseil-cpp\\absl\\strings\\internal\\utf8.cc " +
     "third_party\\abseil-cpp\\absl\\strings\\match.cc " +
     "third_party\\abseil-cpp\\absl\\strings\\numbers.cc " +
@@ -916,6 +922,7 @@ if (PHP_GRPC != "no") {
   FSO.CreateFolder(base_dir+"\\ext\\grpc\\third_party\\abseil-cpp\\absl\\numeric");
   FSO.CreateFolder(base_dir+"\\ext\\grpc\\third_party\\abseil-cpp\\absl\\strings");
   FSO.CreateFolder(base_dir+"\\ext\\grpc\\third_party\\abseil-cpp\\absl\\strings\\internal");
+  FSO.CreateFolder(base_dir+"\\ext\\grpc\\third_party\\abseil-cpp\\absl\\strings\\internal\\str_format");
   FSO.CreateFolder(base_dir+"\\ext\\grpc\\third_party\\abseil-cpp\\absl\\types");
   FSO.CreateFolder(base_dir+"\\ext\\grpc\\third_party\\address_sorting");
   FSO.CreateFolder(base_dir+"\\ext\\grpc\\third_party\\boringssl-with-bazel");

doc/g_stands_for.md

@@ -26,4 +26,5 @@
 - 1.24 'g' stands for ['ganges'](https://github.com/grpc/grpc/tree/v1.24.x)
 - 1.25 'g' stands for ['game'](https://github.com/grpc/grpc/tree/v1.25.x)
 - 1.26 'g' stands for ['gon'](https://github.com/grpc/grpc/tree/v1.26.x)
-- 1.27 'g' stands for ['guantao'](https://github.com/grpc/grpc/tree/master)
+- 1.27 'g' stands for ['guantao'](https://github.com/grpc/grpc/tree/v1.27.x)
+- 1.28 'g' stands for ['galactic'](https://github.com/grpc/grpc/tree/master)

doc/python/sphinx/_static/custom.css

@@ -0,0 +1,3 @@
+dl.field-list > dt {
+    word-break: keep-all !important;
+}

doc/python/sphinx/conf.py

@@ -16,8 +16,8 @@
 
 import os
 import sys
-PYTHON_FOLDER = os.path.join(os.path.dirname(os.path.realpath(__file__)),
-                             '..', '..', '..', 'src', 'python')
+PYTHON_FOLDER = os.path.join(os.path.dirname(os.path.realpath(__file__)), '..',
+                             '..', '..', 'src', 'python')
 sys.path.insert(0, os.path.join(PYTHON_FOLDER, 'grpcio'))
 sys.path.insert(0, os.path.join(PYTHON_FOLDER, 'grpcio_channelz'))
 sys.path.insert(0, os.path.join(PYTHON_FOLDER, 'grpcio_health_checking'))
@@ -53,6 +53,7 @@ extensions = [
     'sphinx.ext.todo',
     'sphinx.ext.napoleon',
     'sphinx.ext.coverage',
+    'sphinx.ext.autodoc.typehints',
 ]
 
 napoleon_google_docstring = True
@@ -63,15 +64,9 @@ autodoc_default_options = {
     'members': None,
 }
 
-autodoc_mock_imports = [
-    'grpc._cython',
-    'grpc_channelz.v1.channelz_pb2',
-    'grpc_channelz.v1.channelz_pb2_grpc',
-    'grpc_health.v1.health_pb2',
-    'grpc_health.v1.health_pb2_grpc',
-    'grpc_reflection.v1alpha.reflection_pb2',
-    'grpc_reflection.v1alpha.reflection_pb2_grpc',
-]
+autodoc_mock_imports = []
+
+autodoc_typehints = 'description'
 
 # -- HTML Configuration -------------------------------------------------
 
@@ -84,6 +79,7 @@ html_theme_options = {
     'description': grpc_version.VERSION,
     'show_powered_by': False,
 }
+html_static_path = ["_static"]
 
 # -- Options for manual page output ------------------------------------------
 

doc/python/sphinx/grpc_asyncio.rst

@@ -0,0 +1,132 @@
+gRPC AsyncIO API
+================
+
+.. module:: grpc.experimental.aio
+
+Overview
+--------
+
+gRPC AsyncIO API is the **new version** of gRPC Python whose architecture is
+tailored to AsyncIO. Underlying, it utilizes the same C-extension, gRPC C-Core,
+as existing stack, and it replaces all gRPC IO operations with methods provided
+by the AsyncIO library.
+
+This stack currently is under active development. Feel free to offer
+suggestions by opening issues on our GitHub repo `grpc/grpc <https://github.com/grpc/grpc>`_.
+
+The design doc can be found here as `gRFC <https://github.com/grpc/proposal/pull/155>`_.
+
+
+Caveats
+-------
+
+gRPC Async API objects may only be used on the thread on which they were
+created. AsyncIO doesn't provide thread safety for most of its APIs.
+
+
+Module Contents
+---------------
+
+Enable AsyncIO in gRPC
+^^^^^^^^^^^^^^^^^^^^^^
+
+.. function:: init_grpc_aio
+
+    Enable AsyncIO for gRPC Python.
+
+    This function is idempotent and it should be invoked before creation of
+    AsyncIO stack objects. Otherwise, the application might deadlock.
+
+    This function configurates the gRPC C-Core to invoke AsyncIO methods for IO
+    operations (e.g., socket read, write). The configuration applies to the
+    entire process.
+
+    After invoking this function, making blocking function calls in coroutines
+    or in the thread running event loop will block the event loop, potentially
+    starving all RPCs in the process. Refer to the Python language
+    documentation on AsyncIO for more details (`running-blocking-code <https://docs.python.org/3/library/asyncio-dev.html#running-blocking-code>`_).
+
+
+Create Channel
+^^^^^^^^^^^^^^
+
+Channels are the abstraction of clients, where most of networking logic
+happens, for example, managing one or more underlying connections, name
+resolution, load balancing, flow control, etc.. If you are using ProtoBuf,
+Channel objects works best when further encapsulate into stub objects, then the
+application can invoke remote functions as if they are local functions.
+
+.. autofunction:: insecure_channel
+.. autofunction:: secure_channel
+
+
+Channel Object
+^^^^^^^^^^^^^^
+
+.. autoclass:: Channel
+
+
+Create Server
+^^^^^^^^^^^^^
+
+.. autofunction:: server
+
+
+Server Object
+^^^^^^^^^^^^^
+
+.. autoclass:: Server
+
+
+gRPC Exceptions
+^^^^^^^^^^^^^^^
+
+.. autoexception:: BaseError
+.. autoexception:: UsageError
+.. autoexception:: AbortError
+.. autoexception:: InternalError
+.. autoexception:: AioRpcError
+
+
+Shared Context
+^^^^^^^^^^^^^^^^^^^^
+
+.. autoclass:: RpcContext
+
+
+Client-Side Context
+^^^^^^^^^^^^^^^^^^^^^^^
+
+.. autoclass:: Call
+.. autoclass:: UnaryUnaryCall
+.. autoclass:: UnaryStreamCall
+.. autoclass:: StreamUnaryCall
+.. autoclass:: StreamStreamCall
+
+
+Server-Side Context
+^^^^^^^^^^^^^^^^^^^^^^^
+
+.. autoclass:: ServicerContext
+
+
+Client-Side Interceptor
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+.. autoclass:: ClientCallDetails
+.. autoclass:: InterceptedUnaryUnaryCall
+.. autoclass:: UnaryUnaryClientInterceptor
+
+.. Service-Side Context
+.. ^^^^^^^^^^^^^^^^^^^^
+
+.. .. autoclass:: ServicerContext
+
+
+Multi-Callable Interfaces
+^^^^^^^^^^^^^^^^^^^^^^^^^
+
+.. autoclass:: UnaryUnaryMultiCallable
+.. autoclass:: UnaryStreamMultiCallable()
+.. autoclass:: StreamUnaryMultiCallable()
+.. autoclass:: StreamStreamMultiCallable()

doc/python/sphinx/index.rst

@@ -10,6 +10,7 @@ API Reference
    :caption: Contents:
 
    grpc
+   grpc_asyncio
    grpc_channelz
    grpc_health_checking
    grpc_reflection

gRPC-C++.podspec

@@ -22,7 +22,7 @@
 Pod::Spec.new do |s|
   s.name     = 'gRPC-C++'
   # TODO (mxyan): use version that match gRPC version when pod is stabilized
-  version = '1.27.0-dev'
+  version = '1.28.0-dev'
   s.version  = version
   s.summary  = 'gRPC C++ library'
   s.homepage = 'https://grpc.io'
@@ -215,6 +215,8 @@ Pod::Spec.new do |s|
     ss.dependency 'gRPC-Core', version
     abseil_version = '0.20190808.1'
     ss.dependency 'abseil/container/inlined_vector', abseil_version
+    ss.dependency 'abseil/memory/memory', abseil_version
+    ss.dependency 'abseil/strings/str_format', abseil_version
     ss.dependency 'abseil/strings/strings', abseil_version
     ss.dependency 'abseil/types/optional', abseil_version
 

gRPC-Core.podspec

@@ -21,7 +21,7 @@
 
 Pod::Spec.new do |s|
   s.name     = 'gRPC-Core'
-  version = '1.27.0-dev'
+  version = '1.28.0-dev'
   s.version  = version
   s.summary  = 'Core cross-platform gRPC library, written in C'
   s.homepage = 'https://grpc.io'
@@ -175,6 +175,8 @@ Pod::Spec.new do |s|
     ss.dependency 'BoringSSL-GRPC', '0.0.7'
     abseil_version = '0.20190808.1'
     ss.dependency 'abseil/container/inlined_vector', abseil_version
+    ss.dependency 'abseil/memory/memory', abseil_version
+    ss.dependency 'abseil/strings/str_format', abseil_version
     ss.dependency 'abseil/strings/strings', abseil_version
     ss.dependency 'abseil/types/optional', abseil_version
     ss.compiler_flags = '-DGRPC_SHADOW_BORINGSSL_SYMBOLS'

gRPC-ProtoRPC.podspec

@@ -21,7 +21,7 @@
 
 Pod::Spec.new do |s|
   s.name     = 'gRPC-ProtoRPC'
-  version = '1.27.0-dev'
+  version = '1.28.0-dev'
   s.version  = version
   s.summary  = 'RPC library for Protocol Buffers, based on gRPC'
   s.homepage = 'https://grpc.io'

gRPC-RxLibrary.podspec

@@ -21,7 +21,7 @@
 
 Pod::Spec.new do |s|
   s.name     = 'gRPC-RxLibrary'
-  version = '1.27.0-dev'
+  version = '1.28.0-dev'
   s.version  = version
   s.summary  = 'Reactive Extensions library for iOS/OSX.'
   s.homepage = 'https://grpc.io'

gRPC.podspec

@@ -20,7 +20,7 @@
 
 Pod::Spec.new do |s|
   s.name     = 'gRPC'
-  version = '1.27.0-dev'
+  version = '1.28.0-dev'
   s.version  = version
   s.summary  = 'gRPC client library for iOS/OSX'
   s.homepage = 'https://grpc.io'

grpc.gemspec

@@ -965,6 +965,19 @@ Gem::Specification.new do |s|
   s.files += %w( third_party/abseil-cpp/absl/strings/internal/ostringstream.h )
   s.files += %w( third_party/abseil-cpp/absl/strings/internal/resize_uninitialized.h )
   s.files += %w( third_party/abseil-cpp/absl/strings/internal/stl_type_traits.h )
+  s.files += %w( third_party/abseil-cpp/absl/strings/internal/str_format/arg.cc )
+  s.files += %w( third_party/abseil-cpp/absl/strings/internal/str_format/arg.h )
+  s.files += %w( third_party/abseil-cpp/absl/strings/internal/str_format/bind.cc )
+  s.files += %w( third_party/abseil-cpp/absl/strings/internal/str_format/bind.h )
+  s.files += %w( third_party/abseil-cpp/absl/strings/internal/str_format/checker.h )
+  s.files += %w( third_party/abseil-cpp/absl/strings/internal/str_format/extension.cc )
+  s.files += %w( third_party/abseil-cpp/absl/strings/internal/str_format/extension.h )
+  s.files += %w( third_party/abseil-cpp/absl/strings/internal/str_format/float_conversion.cc )
+  s.files += %w( third_party/abseil-cpp/absl/strings/internal/str_format/float_conversion.h )
+  s.files += %w( third_party/abseil-cpp/absl/strings/internal/str_format/output.cc )
+  s.files += %w( third_party/abseil-cpp/absl/strings/internal/str_format/output.h )
+  s.files += %w( third_party/abseil-cpp/absl/strings/internal/str_format/parser.cc )
+  s.files += %w( third_party/abseil-cpp/absl/strings/internal/str_format/parser.h )
   s.files += %w( third_party/abseil-cpp/absl/strings/internal/str_join_internal.h )
   s.files += %w( third_party/abseil-cpp/absl/strings/internal/str_split_internal.h )
   s.files += %w( third_party/abseil-cpp/absl/strings/internal/utf8.cc )
@@ -975,6 +988,7 @@ Gem::Specification.new do |s|
   s.files += %w( third_party/abseil-cpp/absl/strings/numbers.h )
   s.files += %w( third_party/abseil-cpp/absl/strings/str_cat.cc )
   s.files += %w( third_party/abseil-cpp/absl/strings/str_cat.h )
+  s.files += %w( third_party/abseil-cpp/absl/strings/str_format.h )
   s.files += %w( third_party/abseil-cpp/absl/strings/str_join.h )
   s.files += %w( third_party/abseil-cpp/absl/strings/str_replace.cc )
   s.files += %w( third_party/abseil-cpp/absl/strings/str_replace.h )

grpc.gyp

@@ -442,6 +442,8 @@
       'type': 'static_library',
       'dependencies': [
         'absl/container:inlined_vector',
+        'absl/memory:memory',
+        'absl/strings:str_format',
         'absl/strings:strings',
         'absl/types:optional',
       ],

include/grpcpp/impl/codegen/completion_queue_impl.h

@@ -387,12 +387,14 @@ class CompletionQueue : private ::grpc::GrpcLibraryCodegen {
   }
 
   void RegisterServer(const Server* server) {
+    (void)server;
 #ifndef NDEBUG
     grpc::internal::MutexLock l(&server_list_mutex_);
     server_list_.push_back(server);
 #endif
   }
   void UnregisterServer(const Server* server) {
+    (void)server;
 #ifndef NDEBUG
     grpc::internal::MutexLock l(&server_list_mutex_);
     server_list_.remove(server);

include/grpcpp/impl/codegen/server_callback_handlers.h

@@ -117,9 +117,19 @@ class CallbackUnaryHandler : public ::grpc::internal::MethodHandler {
   class ServerCallbackUnaryImpl : public ServerCallbackUnary {
    public:
     void Finish(::grpc::Status s) override {
+      // A callback that only contains a call to MaybeDone can be run as an
+      // inline callback regardless of whether or not OnDone is inlineable
+      // because if the actual OnDone callback needs to be scheduled, MaybeDone
+      // is responsible for dispatching to an executor thread if needed. Thus,
+      // when setting up the finish_tag_, we can set its own callback to
+      // inlineable.
       finish_tag_.Set(
-          call_.call(), [this](bool) { MaybeDone(); }, &finish_ops_,
-          reactor_.load(std::memory_order_relaxed)->InternalInlineable());
+          call_.call(),
+          [this](bool) {
+            this->MaybeDone(
+                reactor_.load(std::memory_order_relaxed)->InternalInlineable());
+          },
+          &finish_ops_, /*can_inline=*/true);
       finish_ops_.set_core_cq_tag(&finish_tag_);
 
       if (!ctx_->sent_initial_metadata_) {
@@ -144,13 +154,19 @@ class CallbackUnaryHandler : public ::grpc::internal::MethodHandler {
     void SendInitialMetadata() override {
       GPR_CODEGEN_ASSERT(!ctx_->sent_initial_metadata_);
       this->Ref();
+      // The callback for this function should not be marked inline because it
+      // is directly invoking a user-controlled reaction
+      // (OnSendInitialMetadataDone). Thus it must be dispatched to an executor
+      // thread. However, any OnDone needed after that can be inlined because it
+      // is already running on an executor thread.
       meta_tag_.Set(call_.call(),
                     [this](bool ok) {
-                      reactor_.load(std::memory_order_relaxed)
-                          ->OnSendInitialMetadataDone(ok);
-                      MaybeDone();
+                      ServerUnaryReactor* reactor =
+                          reactor_.load(std::memory_order_relaxed);
+                      reactor->OnSendInitialMetadataDone(ok);
+                      this->MaybeDone(/*inlineable_ondone=*/true);
                     },
-                    &meta_ops_, false);
+                    &meta_ops_, /*can_inline=*/false);
       meta_ops_.SendInitialMetadata(&ctx_->initial_metadata_,
                                     ctx_->initial_metadata_flags());
       if (ctx_->compression_level_set()) {
@@ -184,22 +200,20 @@ class CallbackUnaryHandler : public ::grpc::internal::MethodHandler {
       reactor_.store(reactor, std::memory_order_relaxed);
       this->BindReactor(reactor);
       this->MaybeCallOnCancel(reactor);
-      this->MaybeDone();
+      this->MaybeDone(reactor->InternalInlineable());
     }
 
     const RequestType* request() { return allocator_state_->request(); }
     ResponseType* response() { return allocator_state_->response(); }
 
-    void MaybeDone() override {
-      if (GPR_UNLIKELY(this->Unref() == 1)) {
-        reactor_.load(std::memory_order_relaxed)->OnDone();
-        grpc_call* call = call_.call();
-        auto call_requester = std::move(call_requester_);
-        allocator_state_->Release();
-        this->~ServerCallbackUnaryImpl();  // explicitly call destructor
-        ::grpc::g_core_codegen_interface->grpc_call_unref(call);
-        call_requester();
-      }
+    void CallOnDone() override {
+      reactor_.load(std::memory_order_relaxed)->OnDone();
+      grpc_call* call = call_.call();
+      auto call_requester = std::move(call_requester_);
+      allocator_state_->Release();
+      this->~ServerCallbackUnaryImpl();  // explicitly call destructor
+      ::grpc::g_core_codegen_interface->grpc_call_unref(call);
+      call_requester();
     }
 
     ServerReactor* reactor() override {
@@ -255,8 +269,13 @@ class CallbackClientStreamingHandler : public ::grpc::internal::MethodHandler {
             static_cast<::grpc_impl::CallbackServerContext*>(
                 param.server_context),
             param.call, std::move(param.call_requester));
+    // Inlineable OnDone can be false in the CompletionOp callback because there
+    // is no read reactor that has an inlineable OnDone; this only applies to
+    // the DefaultReactor (which is unary).
     param.server_context->BeginCompletionOp(
-        param.call, [reader](bool) { reader->MaybeDone(); }, reader);
+        param.call,
+        [reader](bool) { reader->MaybeDone(/*inlineable_ondone=*/false); },
+        reader);
 
     ServerReadReactor<RequestType>* reactor = nullptr;
     if (param.status.ok()) {
@@ -287,8 +306,17 @@ class CallbackClientStreamingHandler : public ::grpc::internal::MethodHandler {
   class ServerCallbackReaderImpl : public ServerCallbackReader<RequestType> {
    public:
     void Finish(::grpc::Status s) override {
-      finish_tag_.Set(call_.call(), [this](bool) { MaybeDone(); }, &finish_ops_,
-                      false);
+      // A finish tag with only MaybeDone can have its callback inlined
+      // regardless even if OnDone is not inlineable because this callback just
+      // checks a ref and then decides whether or not to dispatch OnDone.
+      finish_tag_.Set(call_.call(),
+                      [this](bool) {
+                        // Inlineable OnDone can be false here because there is
+                        // no read reactor that has an inlineable OnDone; this
+                        // only applies to the DefaultReactor (which is unary).
+                        this->MaybeDone(/*inlineable_ondone=*/false);
+                      },
+                      &finish_ops_, /*can_inline=*/true);
       if (!ctx_->sent_initial_metadata_) {
         finish_ops_.SendInitialMetadata(&ctx_->initial_metadata_,
                                         ctx_->initial_metadata_flags());
@@ -311,13 +339,17 @@ class CallbackClientStreamingHandler : public ::grpc::internal::MethodHandler {
     void SendInitialMetadata() override {
       GPR_CODEGEN_ASSERT(!ctx_->sent_initial_metadata_);
       this->Ref();
+      // The callback for this function should not be inlined because it invokes
+      // a user-controlled reaction, but any resulting OnDone can be inlined in
+      // the executor to which this callback is dispatched.
       meta_tag_.Set(call_.call(),
                     [this](bool ok) {
-                      reactor_.load(std::memory_order_relaxed)
-                          ->OnSendInitialMetadataDone(ok);
-                      MaybeDone();
+                      ServerReadReactor<RequestType>* reactor =
+                          reactor_.load(std::memory_order_relaxed);
+                      reactor->OnSendInitialMetadataDone(ok);
+                      this->MaybeDone(/*inlineable_ondone=*/true);
                     },
-                    &meta_ops_, false);
+                    &meta_ops_, /*can_inline=*/false);
       meta_ops_.SendInitialMetadata(&ctx_->initial_metadata_,
                                     ctx_->initial_metadata_flags());
       if (ctx_->compression_level_set()) {
@@ -344,31 +376,35 @@ class CallbackClientStreamingHandler : public ::grpc::internal::MethodHandler {
 
     void SetupReactor(ServerReadReactor<RequestType>* reactor) {
       reactor_.store(reactor, std::memory_order_relaxed);
+      // The callback for this function should not be inlined because it invokes
+      // a user-controlled reaction, but any resulting OnDone can be inlined in
+      // the executor to which this callback is dispatched.
       read_tag_.Set(call_.call(),
-                    [this](bool ok) {
-                      reactor_.load(std::memory_order_relaxed)->OnReadDone(ok);
-                      MaybeDone();
+                    [this, reactor](bool ok) {
+                      reactor->OnReadDone(ok);
+                      this->MaybeDone(/*inlineable_ondone=*/true);
                     },
-                    &read_ops_, false);
+                    &read_ops_, /*can_inline=*/false);
       read_ops_.set_core_cq_tag(&read_tag_);
       this->BindReactor(reactor);
       this->MaybeCallOnCancel(reactor);
-      this->MaybeDone();
+      // Inlineable OnDone can be false here because there is no read
+      // reactor that has an inlineable OnDone; this only applies to the
+      // DefaultReactor (which is unary).
+      this->MaybeDone(/*inlineable_ondone=*/false);
     }
 
     ~ServerCallbackReaderImpl() {}
 
     ResponseType* response() { return &resp_; }
 
-    void MaybeDone() override {
-      if (GPR_UNLIKELY(this->Unref() == 1)) {
-        reactor_.load(std::memory_order_relaxed)->OnDone();
-        grpc_call* call = call_.call();
-        auto call_requester = std::move(call_requester_);
-        this->~ServerCallbackReaderImpl();  // explicitly call destructor
-        ::grpc::g_core_codegen_interface->grpc_call_unref(call);
-        call_requester();
-      }
+    void CallOnDone() override {
+      reactor_.load(std::memory_order_relaxed)->OnDone();
+      grpc_call* call = call_.call();
+      auto call_requester = std::move(call_requester_);
+      this->~ServerCallbackReaderImpl();  // explicitly call destructor
+      ::grpc::g_core_codegen_interface->grpc_call_unref(call);
+      call_requester();
     }
 
     ServerReactor* reactor() override {
@@ -419,8 +455,13 @@ class CallbackServerStreamingHandler : public ::grpc::internal::MethodHandler {
                 param.server_context),
             param.call, static_cast<RequestType*>(param.request),
             std::move(param.call_requester));
+    // Inlineable OnDone can be false in the CompletionOp callback because there
+    // is no write reactor that has an inlineable OnDone; this only applies to
+    // the DefaultReactor (which is unary).
     param.server_context->BeginCompletionOp(
-        param.call, [writer](bool) { writer->MaybeDone(); }, writer);
+        param.call,
+        [writer](bool) { writer->MaybeDone(/*inlineable_ondone=*/false); },
+        writer);
 
     ServerWriteReactor<ResponseType>* reactor = nullptr;
     if (param.status.ok()) {
@@ -467,8 +508,17 @@ class CallbackServerStreamingHandler : public ::grpc::internal::MethodHandler {
   class ServerCallbackWriterImpl : public ServerCallbackWriter<ResponseType> {
    public:
     void Finish(::grpc::Status s) override {
-      finish_tag_.Set(call_.call(), [this](bool) { MaybeDone(); }, &finish_ops_,
-                      false);
+      // A finish tag with only MaybeDone can have its callback inlined
+      // regardless even if OnDone is not inlineable because this callback just
+      // checks a ref and then decides whether or not to dispatch OnDone.
+      finish_tag_.Set(call_.call(),
+                      [this](bool) {
+                        // Inlineable OnDone can be false here because there is
+                        // no write reactor that has an inlineable OnDone; this
+                        // only applies to the DefaultReactor (which is unary).
+                        this->MaybeDone(/*inlineable_ondone=*/false);
+                      },
+                      &finish_ops_, /*can_inline=*/true);
       finish_ops_.set_core_cq_tag(&finish_tag_);
 
       if (!ctx_->sent_initial_metadata_) {
@@ -486,13 +536,17 @@ class CallbackServerStreamingHandler : public ::grpc::internal::MethodHandler {
     void SendInitialMetadata() override {
       GPR_CODEGEN_ASSERT(!ctx_->sent_initial_metadata_);
       this->Ref();
+      // The callback for this function should not be inlined because it invokes
+      // a user-controlled reaction, but any resulting OnDone can be inlined in
+      // the executor to which this callback is dispatched.
       meta_tag_.Set(call_.call(),
                     [this](bool ok) {
-                      reactor_.load(std::memory_order_relaxed)
-                          ->OnSendInitialMetadataDone(ok);
-                      MaybeDone();
+                      ServerWriteReactor<ResponseType>* reactor =
+                          reactor_.load(std::memory_order_relaxed);
+                      reactor->OnSendInitialMetadataDone(ok);
+                      this->MaybeDone(/*inlineable_ondone=*/true);
                     },
-                    &meta_ops_, false);
+                    &meta_ops_, /*can_inline=*/false);
       meta_ops_.SendInitialMetadata(&ctx_->initial_metadata_,
                                     ctx_->initial_metadata_flags());
       if (ctx_->compression_level_set()) {
@@ -547,31 +601,34 @@ class CallbackServerStreamingHandler : public ::grpc::internal::MethodHandler {
 
     void SetupReactor(ServerWriteReactor<ResponseType>* reactor) {
       reactor_.store(reactor, std::memory_order_relaxed);
-      write_tag_.Set(
-          call_.call(),
-          [this](bool ok) {
-            reactor_.load(std::memory_order_relaxed)->OnWriteDone(ok);
-            MaybeDone();
-          },
-          &write_ops_, false);
+      // The callback for this function should not be inlined because it invokes
+      // a user-controlled reaction, but any resulting OnDone can be inlined in
+      // the executor to which this callback is dispatched.
+      write_tag_.Set(call_.call(),
+                     [this, reactor](bool ok) {
+                       reactor->OnWriteDone(ok);
+                       this->MaybeDone(/*inlineable_ondone=*/true);
+                     },
+                     &write_ops_, /*can_inline=*/false);
       write_ops_.set_core_cq_tag(&write_tag_);
       this->BindReactor(reactor);
       this->MaybeCallOnCancel(reactor);
-      this->MaybeDone();
+      // Inlineable OnDone can be false here because there is no write
+      // reactor that has an inlineable OnDone; this only applies to the
+      // DefaultReactor (which is unary).
+      this->MaybeDone(/*inlineable_ondone=*/false);
     }
     ~ServerCallbackWriterImpl() { req_->~RequestType(); }
 
     const RequestType* request() { return req_; }
 
-    void MaybeDone() override {
-      if (GPR_UNLIKELY(this->Unref() == 1)) {
-        reactor_.load(std::memory_order_relaxed)->OnDone();
-        grpc_call* call = call_.call();
-        auto call_requester = std::move(call_requester_);
-        this->~ServerCallbackWriterImpl();  // explicitly call destructor
-        ::grpc::g_core_codegen_interface->grpc_call_unref(call);
-        call_requester();
-      }
+    void CallOnDone() override {
+      reactor_.load(std::memory_order_relaxed)->OnDone();
+      grpc_call* call = call_.call();
+      auto call_requester = std::move(call_requester_);
+      this->~ServerCallbackWriterImpl();  // explicitly call destructor
+      ::grpc::g_core_codegen_interface->grpc_call_unref(call);
+      call_requester();
     }
 
     ServerReactor* reactor() override {
@@ -620,8 +677,13 @@ class CallbackBidiHandler : public ::grpc::internal::MethodHandler {
             static_cast<::grpc_impl::CallbackServerContext*>(
                 param.server_context),
             param.call, std::move(param.call_requester));
+    // Inlineable OnDone can be false in the CompletionOp callback because there
+    // is no bidi reactor that has an inlineable OnDone; this only applies to
+    // the DefaultReactor (which is unary).
     param.server_context->BeginCompletionOp(
-        param.call, [stream](bool) { stream->MaybeDone(); }, stream);
+        param.call,
+        [stream](bool) { stream->MaybeDone(/*inlineable_ondone=*/false); },
+        stream);
 
     ServerBidiReactor<RequestType, ResponseType>* reactor = nullptr;
     if (param.status.ok()) {
@@ -652,8 +714,17 @@ class CallbackBidiHandler : public ::grpc::internal::MethodHandler {
       : public ServerCallbackReaderWriter<RequestType, ResponseType> {
    public:
     void Finish(::grpc::Status s) override {
-      finish_tag_.Set(call_.call(), [this](bool) { MaybeDone(); }, &finish_ops_,
-                      false);
+      // A finish tag with only MaybeDone can have its callback inlined
+      // regardless even if OnDone is not inlineable because this callback just
+      // checks a ref and then decides whether or not to dispatch OnDone.
+      finish_tag_.Set(call_.call(),
+                      [this](bool) {
+                        // Inlineable OnDone can be false here because there is
+                        // no bidi reactor that has an inlineable OnDone; this
+                        // only applies to the DefaultReactor (which is unary).
+                        this->MaybeDone(/*inlineable_ondone=*/false);
+                      },
+                      &finish_ops_, /*can_inline=*/true);
       finish_ops_.set_core_cq_tag(&finish_tag_);
 
       if (!ctx_->sent_initial_metadata_) {
@@ -671,13 +742,17 @@ class CallbackBidiHandler : public ::grpc::internal::MethodHandler {
     void SendInitialMetadata() override {
       GPR_CODEGEN_ASSERT(!ctx_->sent_initial_metadata_);
       this->Ref();
+      // The callback for this function should not be inlined because it invokes
+      // a user-controlled reaction, but any resulting OnDone can be inlined in
+      // the executor to which this callback is dispatched.
       meta_tag_.Set(call_.call(),
                     [this](bool ok) {
-                      reactor_.load(std::memory_order_relaxed)
-                          ->OnSendInitialMetadataDone(ok);
-                      MaybeDone();
+                      ServerBidiReactor<RequestType, ResponseType>* reactor =
+                          reactor_.load(std::memory_order_relaxed);
+                      reactor->OnSendInitialMetadataDone(ok);
+                      this->MaybeDone(/*inlineable_ondone=*/true);
                     },
-                    &meta_ops_, false);
+                    &meta_ops_, /*can_inline=*/false);
       meta_ops_.SendInitialMetadata(&ctx_->initial_metadata_,
                                     ctx_->initial_metadata_flags());
       if (ctx_->compression_level_set()) {
@@ -733,35 +808,38 @@ class CallbackBidiHandler : public ::grpc::internal::MethodHandler {
 
     void SetupReactor(ServerBidiReactor<RequestType, ResponseType>* reactor) {
       reactor_.store(reactor, std::memory_order_relaxed);
-      write_tag_.Set(
-          call_.call(),
-          [this](bool ok) {
-            reactor_.load(std::memory_order_relaxed)->OnWriteDone(ok);
-            MaybeDone();
-          },
-          &write_ops_, false);
+      // The callbacks for these functions should not be inlined because they
+      // invoke user-controlled reactions, but any resulting OnDones can be
+      // inlined in the executor to which a callback is dispatched.
+      write_tag_.Set(call_.call(),
+                     [this, reactor](bool ok) {
+                       reactor->OnWriteDone(ok);
+                       this->MaybeDone(/*inlineable_ondone=*/true);
+                     },
+                     &write_ops_, /*can_inline=*/false);
       write_ops_.set_core_cq_tag(&write_tag_);
       read_tag_.Set(call_.call(),
-                    [this](bool ok) {
-                      reactor_.load(std::memory_order_relaxed)->OnReadDone(ok);
-                      MaybeDone();
+                    [this, reactor](bool ok) {
+                      reactor->OnReadDone(ok);
+                      this->MaybeDone(/*inlineable_ondone=*/true);
                     },
-                    &read_ops_, false);
+                    &read_ops_, /*can_inline=*/false);
       read_ops_.set_core_cq_tag(&read_tag_);
       this->BindReactor(reactor);
       this->MaybeCallOnCancel(reactor);
-      this->MaybeDone();
-    }
-
-    void MaybeDone() override {
-      if (GPR_UNLIKELY(this->Unref() == 1)) {
-        reactor_.load(std::memory_order_relaxed)->OnDone();
-        grpc_call* call = call_.call();
-        auto call_requester = std::move(call_requester_);
-        this->~ServerCallbackReaderWriterImpl();  // explicitly call destructor
-        ::grpc::g_core_codegen_interface->grpc_call_unref(call);
-        call_requester();
-      }
+      // Inlineable OnDone can be false here because there is no bidi
+      // reactor that has an inlineable OnDone; this only applies to the
+      // DefaultReactor (which is unary).
+      this->MaybeDone(/*inlineable_ondone=*/false);
+    }
+
+    void CallOnDone() override {
+      reactor_.load(std::memory_order_relaxed)->OnDone();
+      grpc_call* call = call_.call();
+      auto call_requester = std::move(call_requester_);
+      this->~ServerCallbackReaderWriterImpl();  // explicitly call destructor
+      ::grpc::g_core_codegen_interface->grpc_call_unref(call);
+      call_requester();
     }
 
     ServerReactor* reactor() override {

include/grpcpp/impl/codegen/server_callback_impl.h

@@ -73,11 +73,33 @@ class ServerCallbackCall {
  public:
   virtual ~ServerCallbackCall() {}
 
-  // This object is responsible for tracking when it is safe to call
-  // OnCancel. This function should not be called until after the method handler
-  // is done and the RPC has completed with a cancellation. This is tracked by
-  // counting how many of these conditions have been met and calling OnCancel
-  // when none remain unmet.
+  // This object is responsible for tracking when it is safe to call OnDone and
+  // OnCancel. OnDone should not be called until the method handler is complete,
+  // Finish has been called, the ServerContext CompletionOp (which tracks
+  // cancellation or successful completion) has completed, and all outstanding
+  // Read/Write actions have seen their reactions. OnCancel should not be called
+  // until after the method handler is done and the RPC has completed with a
+  // cancellation. This is tracked by counting how many of these conditions have
+  // been met and calling OnCancel when none remain unmet.
+
+  // Public versions of MaybeDone: one where we don't know the reactor in
+  // advance (used for the ServerContext CompletionOp), and one for where we
+  // know the inlineability of the OnDone reaction. You should set the inline
+  // flag to true if either the Reactor is InternalInlineable() or if this
+  // callback is already being forced to run dispatched to an executor
+  // (typically because it contains additional work than just the MaybeDone).
+
+  void MaybeDone() {
+    if (GPR_UNLIKELY(Unref() == 1)) {
+      ScheduleOnDone(reactor()->InternalInlineable());
+    }
+  }
+
+  void MaybeDone(bool inline_ondone) {
+    if (GPR_UNLIKELY(Unref() == 1)) {
+      ScheduleOnDone(inline_ondone);
+    }
+  }
 
   // Fast version called with known reactor passed in, used from derived
   // classes, typically in non-cancel case
@@ -101,14 +123,17 @@ class ServerCallbackCall {
   /// Increases the reference count
   void Ref() { callbacks_outstanding_.fetch_add(1, std::memory_order_relaxed); }
 
-  /// Decreases the reference count and returns the previous value
-  int Unref() {
-    return callbacks_outstanding_.fetch_sub(1, std::memory_order_acq_rel);
-  }
-
  private:
   virtual ServerReactor* reactor() = 0;
-  virtual void MaybeDone() = 0;
+
+  // CallOnDone performs the work required at completion of the RPC: invoking
+  // the OnDone function and doing all necessary cleanup. This function is only
+  // ever invoked on a fully-Unref'fed ServerCallbackCall.
+  virtual void CallOnDone() = 0;
+
+  // If the OnDone reaction is inlineable, execute it inline. Otherwise send it
+  // to an executor.
+  void ScheduleOnDone(bool inline_ondone);
 
   // If the OnCancel reaction is inlineable, execute it inline. Otherwise send
   // it to an executor.
@@ -121,6 +146,11 @@ class ServerCallbackCall {
                1, std::memory_order_acq_rel) == 1;
   }
 
+  /// Decreases the reference count and returns the previous value
+  int Unref() {
+    return callbacks_outstanding_.fetch_sub(1, std::memory_order_acq_rel);
+  }
+
   std::atomic_int on_cancel_conditions_remaining_{2};
   std::atomic_int callbacks_outstanding_{
       3};  // reserve for start, Finish, and CompletionOp

include/grpcpp/impl/codegen/server_context_impl.h

@@ -474,7 +474,7 @@ class ServerContextBase {
     ::grpc::Status status() const { return status_; }
 
    private:
-    void MaybeDone() override {}
+    void CallOnDone() override {}
     ::grpc_impl::internal::ServerReactor* reactor() override {
       return reactor_;
     }

include/grpcpp/security/tls_credentials_options.h

@@ -55,12 +55,13 @@ class TlsKeyMaterialsConfig {
   }
   int version() const { return version_; }
 
-  /** Setter for key materials that will be called by the user. The setter
-   * transfers ownership of the arguments to the config. **/
-  void set_pem_root_certs(grpc::string pem_root_certs);
+  /** Setter for key materials that will be called by the user. Ownership of the
+   * arguments will not be transferred. **/
+  void set_pem_root_certs(const grpc::string& pem_root_certs);
   void add_pem_key_cert_pair(const PemKeyCertPair& pem_key_cert_pair);
-  void set_key_materials(grpc::string pem_root_certs,
-                         std::vector<PemKeyCertPair> pem_key_cert_pair_list);
+  void set_key_materials(
+      const grpc::string& pem_root_certs,
+      const std::vector<PemKeyCertPair>& pem_key_cert_pair_list);
   void set_version(int version) { version_ = version; };
 
  private:
@@ -70,40 +71,36 @@ class TlsKeyMaterialsConfig {
 };
 
 /** TLS credential reload arguments, wraps grpc_tls_credential_reload_arg. It is
- * used for experimental purposes for now and it is subject to change.
+ *  used for experimental purposes for now and it is subject to change.
  *
- * The credential reload arg contains all the info necessary to schedule/cancel
- * a credential reload request. The callback function must be called after
- * finishing the schedule operation. See the description of the
- * grpc_tls_credential_reload_arg struct in grpc_security.h for more details.
+ *  The credential reload arg contains all the info necessary to schedule/cancel
+ *  a credential reload request. The callback function must be called after
+ *  finishing the schedule operation. See the description of the
+ *  grpc_tls_credential_reload_arg struct in grpc_security.h for more details.
  * **/
 class TlsCredentialReloadArg {
  public:
   /** TlsCredentialReloadArg does not take ownership of the C arg that is passed
-   * to the constructor. One must remember to free any memory allocated to the C
-   * arg after using the setter functions below. **/
+   *  to the constructor. One must remember to free any memory allocated to the
+   * C arg after using the setter functions below. **/
   TlsCredentialReloadArg(grpc_tls_credential_reload_arg* arg);
   ~TlsCredentialReloadArg();
 
-  /** Getters for member fields. The callback function is not exposed.
-   * They return the corresponding fields of the underlying C arg. In the case
-   * of the key materials config, it creates a new instance of the C++ key
-   * materials config from the underlying C grpc_tls_key_materials_config. **/
+  /** Getters for member fields. **/
   void* cb_user_data() const;
   bool is_pem_key_cert_pair_list_empty() const;
   grpc_ssl_certificate_config_reload_status status() const;
   grpc::string error_details() const;
 
-  /** Setters for member fields. They modify the fields of the underlying C arg.
-   * The setters for the key_materials_config and the error_details allocate
-   * memory when modifying c_arg_, so one must remember to free c_arg_'s
-   * original key_materials_config or error_details after using the appropriate
-   * setter function.
-   * **/
+  /** Setters for member fields. Ownership of the arguments will not be
+   *  transferred. **/
   void set_cb_user_data(void* cb_user_data);
   void set_pem_root_certs(const grpc::string& pem_root_certs);
   void add_pem_key_cert_pair(
-      TlsKeyMaterialsConfig::PemKeyCertPair pem_key_cert_pair);
+      const TlsKeyMaterialsConfig::PemKeyCertPair& pem_key_cert_pair);
+  void set_key_materials(const grpc::string& pem_root_certs,
+                         std::vector<TlsKeyMaterialsConfig::PemKeyCertPair>
+                             pem_key_cert_pair_list);
   void set_key_materials_config(
       const std::shared_ptr<TlsKeyMaterialsConfig>& key_materials_config);
   void set_status(grpc_ssl_certificate_config_reload_status status);
@@ -187,8 +184,7 @@ class TlsServerAuthorizationCheckArg {
   TlsServerAuthorizationCheckArg(grpc_tls_server_authorization_check_arg* arg);
   ~TlsServerAuthorizationCheckArg();
 
-  /** Getters for member fields. They return the corresponding fields of the
-   * underlying C arg.**/
+  /** Getters for member fields. **/
   void* cb_user_data() const;
   int success() const;
   grpc::string target_name() const;
@@ -197,12 +193,7 @@ class TlsServerAuthorizationCheckArg {
   grpc_status_code status() const;
   grpc::string error_details() const;
 
-  /** Setters for member fields. They modify the fields of the underlying C arg.
-   * The setters for target_name, peer_cert, and error_details allocate memory
-   * when modifying c_arg_, so one must remember to free c_arg_'s original
-   * target_name, peer_cert, or error_details after using the appropriate setter
-   * function.
-   * **/
+  /** Setters for member fields. **/
   void set_cb_user_data(void* cb_user_data);
   void set_success(int success);
   void set_target_name(const grpc::string& target_name);

package.xml

@@ -13,8 +13,8 @@
  <date>2019-09-24</date>
  <time>16:06:07</time>
  <version>
-  <release>1.27.0dev</release>
-  <api>1.27.0dev</api>
+  <release>1.28.0dev</release>
+  <api>1.28.0dev</api>
  </version>
  <stability>
   <release>beta</release>
@@ -22,7 +22,7 @@
  </stability>
  <license>Apache 2.0</license>
  <notes>
-- gRPC Core 1.27.0 update
+- gRPC Core 1.28.0 update
  </notes>
  <contents>
   <dir baseinstalldir="/" name="/">
@@ -970,6 +970,19 @@
     <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/internal/ostringstream.h" role="src" />
     <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/internal/resize_uninitialized.h" role="src" />
     <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/internal/stl_type_traits.h" role="src" />
+    <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/internal/str_format/arg.cc" role="src" />
+    <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/internal/str_format/arg.h" role="src" />
+    <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/internal/str_format/bind.cc" role="src" />
+    <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/internal/str_format/bind.h" role="src" />
+    <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/internal/str_format/checker.h" role="src" />
+    <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/internal/str_format/extension.cc" role="src" />
+    <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/internal/str_format/extension.h" role="src" />
+    <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/internal/str_format/float_conversion.cc" role="src" />
+    <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/internal/str_format/float_conversion.h" role="src" />
+    <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/internal/str_format/output.cc" role="src" />
+    <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/internal/str_format/output.h" role="src" />
+    <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/internal/str_format/parser.cc" role="src" />
+    <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/internal/str_format/parser.h" role="src" />
     <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/internal/str_join_internal.h" role="src" />
     <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/internal/str_split_internal.h" role="src" />
     <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/internal/utf8.cc" role="src" />
@@ -980,6 +993,7 @@
     <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/numbers.h" role="src" />
     <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/str_cat.cc" role="src" />
     <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/str_cat.h" role="src" />
+    <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/str_format.h" role="src" />
     <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/str_join.h" role="src" />
     <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/str_replace.cc" role="src" />
     <file baseinstalldir="/" name="third_party/abseil-cpp/absl/strings/str_replace.h" role="src" />

setup.py

@@ -241,7 +241,7 @@ if "linux" in sys.platform:
 if not "win32" in sys.platform:
   EXTENSION_LIBRARIES += ('m',)
 if "win32" in sys.platform:
-  EXTENSION_LIBRARIES += ('advapi32', 'ws2_32',)
+  EXTENSION_LIBRARIES += ('advapi32', 'ws2_32', 'dbghelp',)
 if BUILD_WITH_SYSTEM_OPENSSL:
   EXTENSION_LIBRARIES += ('ssl', 'crypto',)
 if BUILD_WITH_SYSTEM_ZLIB:

src/core/ext/filters/client_channel/client_channel.cc

@@ -1577,7 +1577,7 @@ void ChannelData::CreateResolvingLoadBalancingPolicyLocked() {
   LoadBalancingPolicy::Args lb_args;
   lb_args.work_serializer = work_serializer_;
   lb_args.channel_control_helper =
-      grpc_core::MakeUnique<ClientChannelControlHelper>(this);
+      absl::make_unique<ClientChannelControlHelper>(this);
   lb_args.args = channel_args_;
   grpc_core::UniquePtr<char> target_uri(gpr_strdup(target_uri_.get()));
   resolving_lb_policy_.reset(new ResolvingLoadBalancingPolicy(
@@ -1852,7 +1852,7 @@ void ChannelData::StartTransportOpLocked(grpc_transport_op* op) {
                                      MemoryOrder::RELEASE);
       chand->UpdateStateAndPickerLocked(
           GRPC_CHANNEL_SHUTDOWN, "shutdown from API",
-          grpc_core::MakeUnique<LoadBalancingPolicy::TransientFailurePicker>(
+          absl::make_unique<LoadBalancingPolicy::TransientFailurePicker>(
               GRPC_ERROR_REF(op->disconnect_with_error)));
     }
   }

src/core/ext/filters/client_channel/http_connect_handshaker.cc

@@ -385,5 +385,5 @@ void grpc_http_connect_register_handshaker_factory() {
   using namespace grpc_core;
   HandshakerRegistry::RegisterHandshakerFactory(
       true /* at_start */, HANDSHAKER_CLIENT,
-      grpc_core::MakeUnique<HttpConnectHandshakerFactory>());
+      absl::make_unique<HttpConnectHandshakerFactory>());
 }

src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc

@@ -712,9 +712,9 @@ void GrpcLb::Helper::UpdateState(grpc_connectivity_state state,
     client_stats = parent_->lb_calld_->client_stats()->Ref();
   }
   parent_->channel_control_helper()->UpdateState(
-      state, grpc_core::MakeUnique<Picker>(parent_.get(), parent_->serverlist_,
-                                           std::move(picker),
-                                           std::move(client_stats)));
+      state,
+      absl::make_unique<Picker>(parent_.get(), parent_->serverlist_,
+                                std::move(picker), std::move(client_stats)));
 }
 
 void GrpcLb::Helper::RequestReresolution() {
@@ -1919,7 +1919,7 @@ bool maybe_add_client_load_reporting_filter(grpc_channel_stack_builder* builder,
 void grpc_lb_policy_grpclb_init() {
   grpc_core::LoadBalancingPolicyRegistry::Builder::
       RegisterLoadBalancingPolicyFactory(
-          grpc_core::MakeUnique<grpc_core::GrpcLbFactory>());
+          absl::make_unique<grpc_core::GrpcLbFactory>());
   grpc_channel_init_register_stage(GRPC_CLIENT_SUBCHANNEL,
                                    GRPC_CHANNEL_INIT_BUILTIN_PRIORITY,
                                    maybe_add_client_load_reporting_filter,

src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc

@@ -201,7 +201,7 @@ void PickFirst::AttemptToConnectUsingLatestUpdateArgsLocked() {
                            GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE);
     channel_control_helper()->UpdateState(
         GRPC_CHANNEL_TRANSIENT_FAILURE,
-        grpc_core::MakeUnique<TransientFailurePicker>(error));
+        absl::make_unique<TransientFailurePicker>(error));
     return;
   }
   // If one of the subchannels in the new list is already in state
@@ -319,10 +319,10 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked(
             GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE);
         p->channel_control_helper()->UpdateState(
             GRPC_CHANNEL_TRANSIENT_FAILURE,
-            grpc_core::MakeUnique<TransientFailurePicker>(error));
+            absl::make_unique<TransientFailurePicker>(error));
       } else {
         p->channel_control_helper()->UpdateState(
-            GRPC_CHANNEL_CONNECTING, grpc_core::MakeUnique<QueuePicker>(p->Ref(
+            GRPC_CHANNEL_CONNECTING, absl::make_unique<QueuePicker>(p->Ref(
                                          DEBUG_LOCATION, "QueuePicker")));
       }
     } else {
@@ -338,7 +338,7 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked(
         p->selected_ = nullptr;
         p->subchannel_list_.reset();
         p->channel_control_helper()->UpdateState(
-            GRPC_CHANNEL_IDLE, grpc_core::MakeUnique<QueuePicker>(
+            GRPC_CHANNEL_IDLE, absl::make_unique<QueuePicker>(
                                    p->Ref(DEBUG_LOCATION, "QueuePicker")));
       } else {
         // This is unlikely but can happen when a subchannel has been asked
@@ -347,10 +347,10 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked(
         if (connectivity_state == GRPC_CHANNEL_READY) {
           p->channel_control_helper()->UpdateState(
               GRPC_CHANNEL_READY,
-              grpc_core::MakeUnique<Picker>(subchannel()->Ref()));
+              absl::make_unique<Picker>(subchannel()->Ref()));
         } else {  // CONNECTING
           p->channel_control_helper()->UpdateState(
-              connectivity_state, grpc_core::MakeUnique<QueuePicker>(
+              connectivity_state, absl::make_unique<QueuePicker>(
                                       p->Ref(DEBUG_LOCATION, "QueuePicker")));
         }
       }
@@ -395,7 +395,7 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked(
               GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE);
           p->channel_control_helper()->UpdateState(
               GRPC_CHANNEL_TRANSIENT_FAILURE,
-              grpc_core::MakeUnique<TransientFailurePicker>(error));
+              absl::make_unique<TransientFailurePicker>(error));
         }
       }
       sd->CheckConnectivityStateAndStartWatchingLocked();
@@ -406,7 +406,7 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked(
       // Only update connectivity state in case 1.
       if (subchannel_list() == p->subchannel_list_.get()) {
         p->channel_control_helper()->UpdateState(
-            GRPC_CHANNEL_CONNECTING, grpc_core::MakeUnique<QueuePicker>(p->Ref(
+            GRPC_CHANNEL_CONNECTING, absl::make_unique<QueuePicker>(p->Ref(
                                          DEBUG_LOCATION, "QueuePicker")));
       }
       break;
@@ -446,7 +446,7 @@ void PickFirst::PickFirstSubchannelData::ProcessUnselectedReadyLocked() {
   }
   p->selected_ = this;
   p->channel_control_helper()->UpdateState(
-      GRPC_CHANNEL_READY, grpc_core::MakeUnique<Picker>(subchannel()->Ref()));
+      GRPC_CHANNEL_READY, absl::make_unique<Picker>(subchannel()->Ref()));
   for (size_t i = 0; i < subchannel_list()->num_subchannels(); ++i) {
     if (i != Index()) {
       subchannel_list()->subchannel(i)->ShutdownLocked();
@@ -503,7 +503,7 @@ class PickFirstFactory : public LoadBalancingPolicyFactory {
 void grpc_lb_policy_pick_first_init() {
   grpc_core::LoadBalancingPolicyRegistry::Builder::
       RegisterLoadBalancingPolicyFactory(
-          grpc_core::MakeUnique<grpc_core::PickFirstFactory>());
+          absl::make_unique<grpc_core::PickFirstFactory>());
 }
 
 void grpc_lb_policy_pick_first_shutdown() {}

src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc

@@ -322,12 +322,12 @@ void RoundRobin::RoundRobinSubchannelList::
   if (num_ready_ > 0) {
     /* 1) READY */
     p->channel_control_helper()->UpdateState(
-        GRPC_CHANNEL_READY, grpc_core::MakeUnique<Picker>(p, this));
+        GRPC_CHANNEL_READY, absl::make_unique<Picker>(p, this));
   } else if (num_connecting_ > 0) {
     /* 2) CONNECTING */
     p->channel_control_helper()->UpdateState(
-        GRPC_CHANNEL_CONNECTING, grpc_core::MakeUnique<QueuePicker>(
-                                     p->Ref(DEBUG_LOCATION, "QueuePicker")));
+        GRPC_CHANNEL_CONNECTING,
+        absl::make_unique<QueuePicker>(p->Ref(DEBUG_LOCATION, "QueuePicker")));
   } else if (num_transient_failure_ == num_subchannels()) {
     /* 3) TRANSIENT_FAILURE */
     grpc_error* error =
@@ -336,7 +336,7 @@ void RoundRobin::RoundRobinSubchannelList::
                            GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE);
     p->channel_control_helper()->UpdateState(
         GRPC_CHANNEL_TRANSIENT_FAILURE,
-        grpc_core::MakeUnique<TransientFailurePicker>(error));
+        absl::make_unique<TransientFailurePicker>(error));
   }
 }
 
@@ -453,7 +453,7 @@ void RoundRobin::UpdateLocked(UpdateArgs args) {
                            GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE);
     channel_control_helper()->UpdateState(
         GRPC_CHANNEL_TRANSIENT_FAILURE,
-        grpc_core::MakeUnique<TransientFailurePicker>(error));
+        absl::make_unique<TransientFailurePicker>(error));
     subchannel_list_ = std::move(latest_pending_subchannel_list_);
   } else if (subchannel_list_ == nullptr) {
     // If there is no current list, immediately promote the new list to
@@ -498,7 +498,7 @@ class RoundRobinFactory : public LoadBalancingPolicyFactory {
 void grpc_lb_policy_round_robin_init() {
   grpc_core::LoadBalancingPolicyRegistry::Builder::
       RegisterLoadBalancingPolicyFactory(
-          grpc_core::MakeUnique<grpc_core::RoundRobinFactory>());
+          absl::make_unique<grpc_core::RoundRobinFactory>());
 }
 
 void grpc_lb_policy_round_robin_shutdown() {}

src/core/ext/filters/client_channel/lb_policy/xds/cds.cc

@@ -148,7 +148,7 @@ void CdsLb::ClusterWatcher::OnClusterChanged(XdsApi::CdsUpdate cluster_data) {
     LoadBalancingPolicy::Args args;
     args.work_serializer = parent_->work_serializer();
     args.args = parent_->args_;
-    args.channel_control_helper = grpc_core::MakeUnique<Helper>(parent_->Ref());
+    args.channel_control_helper = absl::make_unique<Helper>(parent_->Ref());
     parent_->child_policy_ =
         LoadBalancingPolicyRegistry::CreateLoadBalancingPolicy(
             "xds_experimental", std::move(args));
@@ -173,7 +173,7 @@ void CdsLb::ClusterWatcher::OnError(grpc_error* error) {
   if (parent_->child_policy_ == nullptr) {
     parent_->channel_control_helper()->UpdateState(
         GRPC_CHANNEL_TRANSIENT_FAILURE,
-        grpc_core::MakeUnique<TransientFailurePicker>(error));
+        absl::make_unique<TransientFailurePicker>(error));
   } else {
     GRPC_ERROR_UNREF(error);
   }
@@ -273,7 +273,7 @@ void CdsLb::UpdateLocked(UpdateArgs args) {
       xds_client_->CancelClusterDataWatch(
           StringView(old_config->cluster().c_str()), cluster_watcher_);
     }
-    auto watcher = grpc_core::MakeUnique<ClusterWatcher>(Ref());
+    auto watcher = absl::make_unique<ClusterWatcher>(Ref());
     cluster_watcher_ = watcher.get();
     xds_client_->WatchClusterData(StringView(config_->cluster().c_str()),
                                   std::move(watcher));
@@ -335,7 +335,7 @@ class CdsFactory : public LoadBalancingPolicyFactory {
 void grpc_lb_policy_cds_init() {
   grpc_core::LoadBalancingPolicyRegistry::Builder::
       RegisterLoadBalancingPolicyFactory(
-          grpc_core::MakeUnique<grpc_core::CdsFactory>());
+          absl::make_unique<grpc_core::CdsFactory>());
 }
 
 void grpc_lb_policy_cds_shutdown() {}

src/core/ext/filters/client_channel/lb_policy/xds/xds.cc

@@ -122,31 +122,42 @@ class XdsLb : public LoadBalancingPolicy {
  private:
   class EndpointWatcher;
 
-  // We need this wrapper for the following reasons:
-  // 1. To process per-locality load reporting.
-  // 2. Since pickers are std::unique_ptrs we use this RefCounted wrapper to
-  // control
-  //     references to it by the xds picker and the locality.
-  class EndpointPickerWrapper : public RefCounted<EndpointPickerWrapper> {
+  // A simple wrapper to convert the picker returned from a locality's child
+  // policy as a unique_ptr<> to a RefCountedPtr<>.  This allows it to be
+  // referenced by both the picker and the locality.
+  class RefCountedEndpointPicker : public RefCounted<RefCountedEndpointPicker> {
    public:
-    EndpointPickerWrapper(
-        std::unique_ptr<SubchannelPicker> picker,
-        RefCountedPtr<XdsClientStats::LocalityStats> locality_stats)
+    explicit RefCountedEndpointPicker(std::unique_ptr<SubchannelPicker> picker)
+        : picker_(std::move(picker)) {}
+    PickResult Pick(PickArgs args) { return picker_->Pick(std::move(args)); }
+
+   private:
+    std::unique_ptr<SubchannelPicker> picker_;
+  };
+
+  // A picker that wraps the RefCountedEndpointPicker and performs load
+  // reporting for the locality.
+  class LoadReportingPicker : public RefCounted<LoadReportingPicker> {
+   public:
+    LoadReportingPicker(RefCountedPtr<RefCountedEndpointPicker> picker,
+                        RefCountedPtr<XdsClusterLocalityStats> locality_stats)
         : picker_(std::move(picker)),
-          locality_stats_(std::move(locality_stats)) {
-      locality_stats_->RefByPicker();
-    }
-    ~EndpointPickerWrapper() { locality_stats_->UnrefByPicker(); }
+          locality_stats_(std::move(locality_stats)) {}
 
     PickResult Pick(PickArgs args);
 
+    RefCountedEndpointPicker* picker() const { return picker_.get(); }
+    XdsClusterLocalityStats* locality_stats() const {
+      return locality_stats_.get();
+    }
+
    private:
-    std::unique_ptr<SubchannelPicker> picker_;
-    RefCountedPtr<XdsClientStats::LocalityStats> locality_stats_;
+    RefCountedPtr<RefCountedEndpointPicker> picker_;
+    RefCountedPtr<XdsClusterLocalityStats> locality_stats_;
   };
 
-  // The picker will use a stateless weighting algorithm to pick the locality to
-  // use for each request.
+  // A picker that uses a stateless weighting algorithm to pick the locality
+  // to use for each request.
   class LocalityPicker : public SubchannelPicker {
    public:
     // Maintains a weighted list of pickers from each locality that is in ready
@@ -154,14 +165,12 @@ class XdsLb : public LoadBalancingPolicy {
     // proportional to the locality's weight. The start of the range is the
     // previous value in the vector and is 0 for the first element.
     using PickerList =
-        InlinedVector<std::pair<uint32_t, RefCountedPtr<EndpointPickerWrapper>>,
+        InlinedVector<std::pair<uint32_t, RefCountedPtr<LoadReportingPicker>>,
                       1>;
-    LocalityPicker(RefCountedPtr<XdsLb> xds_policy, PickerList pickers)
-        : xds_policy_(std::move(xds_policy)),
-          pickers_(std::move(pickers)),
-          drop_config_(xds_policy_->drop_config_) {}
-
-    ~LocalityPicker() { xds_policy_.reset(DEBUG_LOCATION, "LocalityPicker"); }
+    LocalityPicker(XdsLb* xds_policy, PickerList pickers)
+        : drop_stats_(xds_policy->drop_stats_),
+          drop_config_(xds_policy->drop_config_),
+          pickers_(std::move(pickers)) {}
 
     PickResult Pick(PickArgs args) override;
 
@@ -169,9 +178,9 @@ class XdsLb : public LoadBalancingPolicy {
     // Calls the picker of the locality that the key falls within.
     PickResult PickFromLocality(const uint32_t key, PickArgs args);
 
-    RefCountedPtr<XdsLb> xds_policy_;
-    PickerList pickers_;
+    RefCountedPtr<XdsClusterDropStats> drop_stats_;
     RefCountedPtr<XdsApi::DropConfig> drop_config_;
+    PickerList pickers_;
   };
 
   class FallbackHelper : public ChannelControlHelper {
@@ -208,18 +217,28 @@ class XdsLb : public LoadBalancingPolicy {
                RefCountedPtr<XdsLocalityName> name);
       ~Locality();
 
-      void UpdateLocked(uint32_t locality_weight, ServerAddressList serverlist);
+      void UpdateLocked(uint32_t locality_weight, ServerAddressList serverlist,
+                        bool update_locality_stats);
       void ShutdownLocked();
       void ResetBackoffLocked();
       void DeactivateLocked();
       void Orphan() override;
 
+      uint32_t weight() const { return weight_; }
+
       grpc_connectivity_state connectivity_state() const {
         return connectivity_state_;
       }
-      uint32_t weight() const { return weight_; }
-      RefCountedPtr<EndpointPickerWrapper> picker_wrapper() const {
-        return picker_wrapper_;
+
+      RefCountedPtr<LoadReportingPicker> GetLoadReportingPicker() {
+        // Recreate load reporting picker if stats object has changed.
+        if (load_reporting_picker_ == nullptr ||
+            load_reporting_picker_->picker() != picker_wrapper_.get() ||
+            load_reporting_picker_->locality_stats() != stats_.get()) {
+          load_reporting_picker_ =
+              MakeRefCounted<LoadReportingPicker>(picker_wrapper_, stats_);
+        }
+        return load_reporting_picker_;
       }
 
       void set_locality_map(RefCountedPtr<LocalityMap> locality_map) {
@@ -258,6 +277,8 @@ class XdsLb : public LoadBalancingPolicy {
       grpc_channel_args* CreateChildPolicyArgsLocked(
           const grpc_channel_args* args);
 
+      void UpdateLocalityStats();
+
       static void OnDelayedRemovalTimer(void* arg, grpc_error* error);
       void OnDelayedRemovalTimerLocked(grpc_error* error);
 
@@ -267,9 +288,11 @@ class XdsLb : public LoadBalancingPolicy {
       RefCountedPtr<LocalityMap> locality_map_;
 
       RefCountedPtr<XdsLocalityName> name_;
+      RefCountedPtr<XdsClusterLocalityStats> stats_;
       OrphanablePtr<LoadBalancingPolicy> child_policy_;
       OrphanablePtr<LoadBalancingPolicy> pending_child_policy_;
-      RefCountedPtr<EndpointPickerWrapper> picker_wrapper_;
+      RefCountedPtr<RefCountedEndpointPicker> picker_wrapper_;
+      RefCountedPtr<LoadReportingPicker> load_reporting_picker_;
       grpc_connectivity_state connectivity_state_ = GRPC_CHANNEL_IDLE;
       uint32_t weight_;
 
@@ -285,7 +308,8 @@ class XdsLb : public LoadBalancingPolicy {
     ~LocalityMap() { xds_policy_.reset(DEBUG_LOCATION, "LocalityMap"); }
 
     void UpdateLocked(
-        const XdsApi::PriorityListUpdate::LocalityMap& locality_map_update);
+        const XdsApi::PriorityListUpdate::LocalityMap& locality_map_update,
+        bool update_locality_stats);
     void ResetBackoffLocked();
     void UpdateXdsPickerLocked();
     OrphanablePtr<Locality> ExtractLocalityLocked(
@@ -357,7 +381,7 @@ class XdsLb : public LoadBalancingPolicy {
                                                : xds_client_.get();
   }
 
-  void UpdatePrioritiesLocked();
+  void UpdatePrioritiesLocked(bool update_locality_stats);
   void UpdateXdsPickerLocked();
   void MaybeCreateLocalityMapLocked(uint32_t priority);
   void FailoverOnConnectionFailureLocked();
@@ -435,15 +459,15 @@ class XdsLb : public LoadBalancingPolicy {
   // The config for dropping calls.
   RefCountedPtr<XdsApi::DropConfig> drop_config_;
 
-  // The stats for client-side load reporting.
-  XdsClientStats client_stats_;
+  // Drop stats for client-side load reporting.
+  RefCountedPtr<XdsClusterDropStats> drop_stats_;
 };
 
 //
-// XdsLb::EndpointPickerWrapper
+// XdsLb::LoadReportingPicker
 //
 
-LoadBalancingPolicy::PickResult XdsLb::EndpointPickerWrapper::Pick(
+LoadBalancingPolicy::PickResult XdsLb::LoadReportingPicker::Pick(
     LoadBalancingPolicy::PickArgs args) {
   // Forward the pick to the picker returned from the child policy.
   PickResult result = picker_->Pick(args);
@@ -454,7 +478,7 @@ LoadBalancingPolicy::PickResult XdsLb::EndpointPickerWrapper::Pick(
   // Record a call started.
   locality_stats_->AddCallStarted();
   // Intercept the recv_trailing_metadata op to record call completion.
-  XdsClientStats::LocalityStats* locality_stats =
+  XdsClusterLocalityStats* locality_stats =
       locality_stats_->Ref(DEBUG_LOCATION, "LocalityStats+call").release();
   result.recv_trailing_metadata_ready =
       // Note: This callback does not run in either the control plane
@@ -476,7 +500,7 @@ XdsLb::PickResult XdsLb::LocalityPicker::Pick(PickArgs args) {
   // Handle drop.
   const std::string* drop_category;
   if (drop_config_->ShouldDrop(&drop_category)) {
-    xds_policy_->client_stats_.AddCallDropped(*drop_category);
+    if (drop_stats_ != nullptr) drop_stats_->AddCallDropped(*drop_category);
     PickResult result;
     result.type = PickResult::PICK_COMPLETE;
     return result;
@@ -621,7 +645,7 @@ class XdsLb::EndpointWatcher : public XdsClient::EndpointWatcherInterface {
     }
     // Update the priority list.
     xds_policy_->priority_list_update_ = std::move(update.priority_list_update);
-    xds_policy_->UpdatePrioritiesLocked();
+    xds_policy_->UpdatePrioritiesLocked(false /*update_locality_stats*/);
   }
 
   void OnError(grpc_error* error) override {
@@ -711,6 +735,7 @@ void XdsLb::ShutdownLocked() {
   shutting_down_ = true;
   MaybeCancelFallbackAtStartupChecks();
   priorities_.clear();
+  drop_stats_.reset();
   if (fallback_policy_ != nullptr) {
     grpc_pollset_set_del_pollset_set(fallback_policy_->interested_parties(),
                                      interested_parties());
@@ -728,14 +753,6 @@ void XdsLb::ShutdownLocked() {
   if (xds_client_from_channel_ != nullptr) {
     xds_client()->CancelEndpointDataWatch(StringView(eds_service_name()),
                                           endpoint_watcher_);
-    if (config_->lrs_load_reporting_server_name().has_value()) {
-      // TODO(roth): We should pass the cluster name (in addition to the
-      // eds_service_name) when adding the client stats. To do so, we need to
-      // first find a way to plumb the cluster name down into this LB policy.
-      xds_client()->RemoveClientStats(
-          StringView(config_->lrs_load_reporting_server_name().value().c_str()),
-          StringView(eds_service_name()), &client_stats_);
-    }
     xds_client_from_channel_.reset();
   }
   xds_client_.reset();
@@ -776,8 +793,6 @@ void XdsLb::UpdateLocked(UpdateArgs args) {
   grpc_channel_args_destroy(args_);
   args_ = args.args;
   args.args = nullptr;
-  // Update priority list.
-  UpdatePrioritiesLocked();
   // Update the existing fallback policy. The fallback policy config and/or the
   // fallback addresses may be new.
   if (fallback_policy_ != nullptr) UpdateFallbackPolicyLocked();
@@ -803,6 +818,31 @@ void XdsLb::UpdateLocked(UpdateArgs args) {
     fallback_at_startup_checks_pending_ = true;
     grpc_timer_init(&lb_fallback_timer_, deadline, &lb_on_fallback_);
   }
+  // Update drop stats for load reporting if needed.
+  if (is_initial_update || config_->lrs_load_reporting_server_name() !=
+                               old_config->lrs_load_reporting_server_name()) {
+    drop_stats_.reset();
+    if (config_->lrs_load_reporting_server_name().has_value()) {
+      drop_stats_ = xds_client()->AddClusterDropStats(
+          config_->lrs_load_reporting_server_name().value(),
+          // TODO(roth): We currently hard-code the assumption that
+          // cluster name and EDS service name are the same.  Fix this
+          // as part of refectoring this LB policy.
+          eds_service_name(), eds_service_name());
+    }
+  }
+  // Update priority list.
+  // Note that this comes after updating drop_stats_, since we want that
+  // to be used by any new picker we create here.
+  // No need to do this on the initial update, since there won't be any
+  // priorities to update yet.
+  if (!is_initial_update) {
+    const bool update_locality_stats =
+        config_->lrs_load_reporting_server_name() !=
+            old_config->lrs_load_reporting_server_name() ||
+        strcmp(old_eds_service_name, eds_service_name()) != 0;
+    UpdatePrioritiesLocked(update_locality_stats);
+  }
   // Update endpoint watcher if needed.
   if (is_initial_update ||
       strcmp(old_eds_service_name, eds_service_name()) != 0) {
@@ -810,40 +850,12 @@ void XdsLb::UpdateLocked(UpdateArgs args) {
       xds_client()->CancelEndpointDataWatch(StringView(old_eds_service_name),
                                             endpoint_watcher_);
     }
-    auto watcher = grpc_core::MakeUnique<EndpointWatcher>(
+    auto watcher = absl::make_unique<EndpointWatcher>(
         Ref(DEBUG_LOCATION, "EndpointWatcher"));
     endpoint_watcher_ = watcher.get();
     xds_client()->WatchEndpointData(StringView(eds_service_name()),
                                     std::move(watcher));
   }
-  // Update load reporting if needed.
-  // TODO(roth): Ideally, we should not collect any stats if load reporting
-  // is disabled, which would require changing this code to recreate
-  // all of the pickers whenever load reporting is enabled or disabled
-  // here.
-  if (is_initial_update ||
-      (config_->lrs_load_reporting_server_name().has_value()) !=
-          (old_config->lrs_load_reporting_server_name().has_value()) ||
-      (config_->lrs_load_reporting_server_name().has_value() &&
-       old_config->lrs_load_reporting_server_name().has_value() &&
-       config_->lrs_load_reporting_server_name().value() !=
-           old_config->lrs_load_reporting_server_name().value())) {
-    if (old_config != nullptr &&
-        old_config->lrs_load_reporting_server_name().has_value()) {
-      xds_client()->RemoveClientStats(
-          StringView(
-              old_config->lrs_load_reporting_server_name().value().c_str()),
-          StringView(old_eds_service_name), &client_stats_);
-    }
-    if (config_->lrs_load_reporting_server_name().has_value()) {
-      // TODO(roth): We should pass the cluster name (in addition to the
-      // eds_service_name) when adding the client stats. To do so, we need to
-      // first find a way to plumb the cluster name down into this LB policy.
-      xds_client()->AddClientStats(
-          StringView(config_->lrs_load_reporting_server_name().value().c_str()),
-          StringView(eds_service_name()), &client_stats_);
-    }
-  }
 }
 
 //
@@ -1026,7 +1038,7 @@ void XdsLb::MaybeExitFallbackMode() {
 // priority list-related methods
 //
 
-void XdsLb::UpdatePrioritiesLocked() {
+void XdsLb::UpdatePrioritiesLocked(bool update_locality_stats) {
   // 1. Remove from the priority list the priorities that are not in the update.
   DeactivatePrioritiesLowerThan(priority_list_update_.LowestPriority());
   // 2. Update all the existing priorities.
@@ -1038,7 +1050,7 @@ void XdsLb::UpdatePrioritiesLocked() {
     // Propagate locality_map_update.
     // TODO(juanlishen): Find a clean way to skip duplicate update for a
     // priority.
-    locality_map->UpdateLocked(*locality_map_update);
+    locality_map->UpdateLocked(*locality_map_update, update_locality_stats);
   }
   // 3. Only create a new locality map if all the existing ones have failed.
   if (priorities_.empty() ||
@@ -1050,6 +1062,11 @@ void XdsLb::UpdatePrioritiesLocked() {
     // to be created.
     MaybeCreateLocalityMapLocked(new_priority);
   }
+  // 4. If we updated locality stats and we already have at least one
+  // priority, update the picker to start using the new stats object(s).
+  if (update_locality_stats && !priorities_.empty()) {
+    UpdateXdsPickerLocked();
+  }
 }
 
 void XdsLb::UpdateXdsPickerLocked() {
@@ -1061,7 +1078,7 @@ void XdsLb::UpdateXdsPickerLocked() {
         GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE);
     channel_control_helper()->UpdateState(
         GRPC_CHANNEL_TRANSIENT_FAILURE,
-        grpc_core::MakeUnique<TransientFailurePicker>(error));
+        absl::make_unique<TransientFailurePicker>(error));
     return;
   }
   priorities_[current_priority_]->UpdateXdsPickerLocked();
@@ -1073,7 +1090,8 @@ void XdsLb::MaybeCreateLocalityMapLocked(uint32_t priority) {
   auto new_locality_map =
       new LocalityMap(Ref(DEBUG_LOCATION, "LocalityMap"), priority);
   priorities_.emplace_back(OrphanablePtr<LocalityMap>(new_locality_map));
-  new_locality_map->UpdateLocked(*priority_list_update_.Find(priority));
+  new_locality_map->UpdateLocked(*priority_list_update_.Find(priority),
+                                 false /*update_locality_stats*/);
 }
 
 void XdsLb::FailoverOnConnectionFailureLocked() {
@@ -1154,13 +1172,14 @@ XdsLb::LocalityMap::LocalityMap(RefCountedPtr<XdsLb> xds_policy,
   if (priority_ == 0) {
     xds_policy_->channel_control_helper()->UpdateState(
         GRPC_CHANNEL_CONNECTING,
-        grpc_core::MakeUnique<QueuePicker>(
+        absl::make_unique<QueuePicker>(
             xds_policy_->Ref(DEBUG_LOCATION, "QueuePicker")));
   }
 }
 
 void XdsLb::LocalityMap::UpdateLocked(
-    const XdsApi::PriorityListUpdate::LocalityMap& locality_map_update) {
+    const XdsApi::PriorityListUpdate::LocalityMap& locality_map_update,
+    bool update_locality_stats) {
   if (xds_policy_->shutting_down_) return;
   if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_xds_trace)) {
     gpr_log(GPR_INFO, "[xdslb %p] Start Updating priority %" PRIu32,
@@ -1203,7 +1222,7 @@ void XdsLb::LocalityMap::UpdateLocked(
     // Keep a copy of serverlist in the update so that we can compare it
     // with the future ones.
     locality->UpdateLocked(locality_update.lb_weight,
-                           locality_update.serverlist);
+                           locality_update.serverlist, update_locality_stats);
   }
 }
 
@@ -1218,20 +1237,19 @@ void XdsLb::LocalityMap::UpdateXdsPickerLocked() {
   // weights of all localities.
   LocalityPicker::PickerList picker_list;
   uint32_t end = 0;
-  for (const auto& p : localities_) {
+  for (auto& p : localities_) {
     const auto& locality_name = p.first;
-    const Locality* locality = p.second.get();
+    Locality* locality = p.second.get();
     // Skip the localities that are not in the latest locality map update.
     if (!locality_map_update()->Contains(locality_name)) continue;
     if (locality->connectivity_state() != GRPC_CHANNEL_READY) continue;
     end += locality->weight();
-    picker_list.push_back(std::make_pair(end, locality->picker_wrapper()));
+    picker_list.push_back(
+        std::make_pair(end, locality->GetLoadReportingPicker()));
   }
   xds_policy()->channel_control_helper()->UpdateState(
       GRPC_CHANNEL_READY,
-      grpc_core::MakeUnique<LocalityPicker>(
-          xds_policy_->Ref(DEBUG_LOCATION, "LocalityPicker"),
-          std::move(picker_list)));
+      absl::make_unique<LocalityPicker>(xds_policy(), std::move(picker_list)));
 }
 
 OrphanablePtr<XdsLb::LocalityMap::Locality>
@@ -1454,6 +1472,8 @@ XdsLb::LocalityMap::Locality::Locality(RefCountedPtr<LocalityMap> locality_map,
   // Closure Initialization
   GRPC_CLOSURE_INIT(&on_delayed_removal_timer_, OnDelayedRemovalTimer, this,
                     grpc_schedule_on_exec_ctx);
+  // Initialize locality stats if load reporting is enabled.
+  UpdateLocalityStats();
 }
 
 XdsLb::LocalityMap::Locality::~Locality() {
@@ -1464,6 +1484,19 @@ XdsLb::LocalityMap::Locality::~Locality() {
   locality_map_.reset(DEBUG_LOCATION, "Locality");
 }
 
+void XdsLb::LocalityMap::Locality::UpdateLocalityStats() {
+  stats_.reset();
+  if (xds_policy()->config_->lrs_load_reporting_server_name().has_value()) {
+    stats_ = xds_policy()->xds_client()->AddClusterLocalityStats(
+        xds_policy()->config_->lrs_load_reporting_server_name().value(),
+        // TODO(roth): We currently hard-code the assumption that
+        // cluster name and EDS service name are the same.  Fix this
+        // as part of refectoring this LB policy.
+        xds_policy()->eds_service_name(), xds_policy()->eds_service_name(),
+        name_);
+  }
+}
+
 grpc_channel_args* XdsLb::LocalityMap::Locality::CreateChildPolicyArgsLocked(
     const grpc_channel_args* args_in) {
   const grpc_arg args_to_add[] = {
@@ -1515,13 +1548,16 @@ XdsLb::LocalityMap::Locality::CreateChildPolicyLocked(
 }
 
 void XdsLb::LocalityMap::Locality::UpdateLocked(uint32_t locality_weight,
-                                                ServerAddressList serverlist) {
+                                                ServerAddressList serverlist,
+                                                bool update_locality_stats) {
   if (xds_policy()->shutting_down_) return;
   // Update locality weight.
   weight_ = locality_weight;
   if (delayed_removal_timer_callback_pending_) {
     grpc_timer_cancel(&delayed_removal_timer_);
   }
+  // Update locality stats.
+  if (update_locality_stats) UpdateLocalityStats();
   // Construct update args.
   UpdateArgs update_args;
   update_args.addresses = std::move(serverlist);
@@ -1629,6 +1665,7 @@ void XdsLb::LocalityMap::Locality::ShutdownLocked() {
     gpr_log(GPR_INFO, "[xdslb %p] Locality %p %s: shutting down locality",
             xds_policy(), this, name_->AsHumanReadableString());
   }
+  stats_.reset();
   // Remove the child policy's interested_parties pollset_set from the
   // xDS policy.
   grpc_pollset_set_del_pollset_set(child_policy_->interested_parties(),
@@ -1642,6 +1679,7 @@ void XdsLb::LocalityMap::Locality::ShutdownLocked() {
   }
   // Drop our ref to the child's picker, in case it's holding a ref to
   // the child.
+  load_reporting_picker_.reset();
   picker_wrapper_.reset();
   if (delayed_removal_timer_callback_pending_) {
     grpc_timer_cancel(&delayed_removal_timer_);
@@ -1696,7 +1734,7 @@ void XdsLb::LocalityMap::Locality::OnDelayedRemovalTimerLocked(
 }
 
 //
-// XdsLb::Locality::Helper
+// XdsLb::LocalityMap::Locality::Helper
 //
 
 bool XdsLb::LocalityMap::Locality::Helper::CalledByPendingChild() const {
@@ -1742,16 +1780,10 @@ void XdsLb::LocalityMap::Locality::Helper::UpdateState(
     // This request is from an outdated child, so ignore it.
     return;
   }
-  // Cache the picker and its state in the locality.
-  // TODO(roth): If load reporting is not configured, we should ideally
-  // pass a null LocalityStats ref to the EndpointPickerWrapper and have it
-  // not collect any stats, since they're not going to be used.  This would
-  // require recreating all of the pickers whenever we get a config update.
-  locality_->picker_wrapper_ = MakeRefCounted<EndpointPickerWrapper>(
-      std::move(picker),
-      locality_->xds_policy()->client_stats_.FindLocalityStats(
-          locality_->name_));
+  // Cache the state and picker in the locality.
   locality_->connectivity_state_ = state;
+  locality_->picker_wrapper_ =
+      MakeRefCounted<RefCountedEndpointPicker>(std::move(picker));
   // Notify the locality map.
   locality_->locality_map_->OnLocalityStateUpdateLocked();
 }
@@ -1871,7 +1903,7 @@ class XdsFactory : public LoadBalancingPolicyFactory {
 void grpc_lb_policy_xds_init() {
   grpc_core::LoadBalancingPolicyRegistry::Builder::
       RegisterLoadBalancingPolicyFactory(
-          grpc_core::MakeUnique<grpc_core::XdsFactory>());
+          absl::make_unique<grpc_core::XdsFactory>());
 }
 
 void grpc_lb_policy_xds_shutdown() {}

src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc

@@ -498,7 +498,7 @@ void grpc_resolver_dns_ares_init() {
     }
     grpc_set_resolver_impl(&ares_resolver);
     grpc_core::ResolverRegistry::Builder::RegisterResolverFactory(
-        grpc_core::MakeUnique<grpc_core::AresDnsResolverFactory>());
+        absl::make_unique<grpc_core::AresDnsResolverFactory>());
   } else {
     g_use_ares_dns_resolver = false;
   }

src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_libuv.cc

@@ -169,7 +169,7 @@ class GrpcPolledFdFactoryLibuv : public GrpcPolledFdFactory {
 
 std::unique_ptr<GrpcPolledFdFactory> NewGrpcPolledFdFactory(
     std::shared_ptr<WorkSerializer> /*work_serializer*/) {
-  return MakeUnique<GrpcPolledFdFactoryLibuv>();
+  return absl::make_unique<GrpcPolledFdFactoryLibuv>();
 }
 
 }  // namespace grpc_core

src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_posix.cc

@@ -99,7 +99,7 @@ class GrpcPolledFdFactoryPosix : public GrpcPolledFdFactory {
 
 std::unique_ptr<GrpcPolledFdFactory> NewGrpcPolledFdFactory(
     std::shared_ptr<WorkSerializer> work_serializer) { /* NOLINT */
-  return MakeUnique<GrpcPolledFdFactoryPosix>();
+  return absl::make_unique<GrpcPolledFdFactoryPosix>();
 }
 
 }  // namespace grpc_core

src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_windows.cc

@@ -887,7 +887,8 @@ class GrpcPolledFdFactoryWindows : public GrpcPolledFdFactory {
 
 std::unique_ptr<GrpcPolledFdFactory> NewGrpcPolledFdFactory(
     std::shared_ptr<WorkSerializer> work_serializer) {
-  return MakeUnique<GrpcPolledFdFactoryWindows>(std::move(work_serializer));
+  return absl::make_unique<GrpcPolledFdFactoryWindows>(
+      std::move(work_serializer));
 }
 
 }  // namespace grpc_core

src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc

@@ -184,7 +184,7 @@ static void on_hostbyname_done_locked(void* arg, int status, int /*timeouts*/,
         "request:%p on_hostbyname_done_locked host=%s ARES_SUCCESS", r,
         hr->host);
     if (*r->addresses_out == nullptr) {
-      *r->addresses_out = grpc_core::MakeUnique<ServerAddressList>();
+      *r->addresses_out = absl::make_unique<ServerAddressList>();
     }
     ServerAddressList& addresses = **r->addresses_out;
     for (size_t i = 0; hostent->h_addr_list[i] != nullptr; ++i) {
@@ -481,7 +481,7 @@ static bool inner_resolve_as_ip_literal_locked(
       grpc_parse_ipv6_hostport(hostport->get(), &addr,
                                false /* log errors */)) {
     GPR_ASSERT(*addrs == nullptr);
-    *addrs = grpc_core::MakeUnique<ServerAddressList>();
+    *addrs = absl::make_unique<ServerAddressList>();
     (*addrs)->emplace_back(addr.addr, addr.len, nullptr /* args */);
     return true;
   }
@@ -544,7 +544,7 @@ static bool inner_maybe_resolve_localhost_manually_locked(
   }
   if (gpr_stricmp(host->get(), "localhost") == 0) {
     GPR_ASSERT(*addrs == nullptr);
-    *addrs = grpc_core::MakeUnique<grpc_core::ServerAddressList>();
+    *addrs = absl::make_unique<grpc_core::ServerAddressList>();
     uint16_t numeric_port = grpc_strhtons(port->get());
     // Append the ipv6 loopback address.
     struct sockaddr_in6 ipv6_loopback_addr;

src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc

@@ -302,7 +302,7 @@ void grpc_resolver_dns_native_init() {
   if (gpr_stricmp(resolver.get(), "native") == 0) {
     gpr_log(GPR_DEBUG, "Using native dns resolver");
     grpc_core::ResolverRegistry::Builder::RegisterResolverFactory(
-        grpc_core::MakeUnique<grpc_core::NativeDnsResolverFactory>());
+        absl::make_unique<grpc_core::NativeDnsResolverFactory>());
   } else {
     grpc_core::ResolverRegistry::Builder::InitRegistry();
     grpc_core::ResolverFactory* existing_factory =
@@ -310,7 +310,7 @@ void grpc_resolver_dns_native_init() {
     if (existing_factory == nullptr) {
       gpr_log(GPR_DEBUG, "Using native dns resolver");
       grpc_core::ResolverRegistry::Builder::RegisterResolverFactory(
-          grpc_core::MakeUnique<grpc_core::NativeDnsResolverFactory>());
+          absl::make_unique<grpc_core::NativeDnsResolverFactory>());
     }
   }
 }

src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc

@@ -377,7 +377,7 @@ class FakeResolverFactory : public ResolverFactory {
 
 void grpc_resolver_fake_init() {
   grpc_core::ResolverRegistry::Builder::RegisterResolverFactory(
-      grpc_core::MakeUnique<grpc_core::FakeResolverFactory>());
+      absl::make_unique<grpc_core::FakeResolverFactory>());
 }
 
 void grpc_resolver_fake_shutdown() {}

src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc

@@ -176,12 +176,12 @@ class UnixResolverFactory : public ResolverFactory {
 
 void grpc_resolver_sockaddr_init() {
   grpc_core::ResolverRegistry::Builder::RegisterResolverFactory(
-      grpc_core::MakeUnique<grpc_core::IPv4ResolverFactory>());
+      absl::make_unique<grpc_core::IPv4ResolverFactory>());
   grpc_core::ResolverRegistry::Builder::RegisterResolverFactory(
-      grpc_core::MakeUnique<grpc_core::IPv6ResolverFactory>());
+      absl::make_unique<grpc_core::IPv6ResolverFactory>());
 #ifdef GRPC_HAVE_UNIX_SOCKET
   grpc_core::ResolverRegistry::Builder::RegisterResolverFactory(
-      grpc_core::MakeUnique<grpc_core::UnixResolverFactory>());
+      absl::make_unique<grpc_core::UnixResolverFactory>());
 #endif
 }
 

src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc

@@ -92,7 +92,7 @@ void XdsResolver::StartLocked() {
   grpc_error* error = GRPC_ERROR_NONE;
   xds_client_ = MakeOrphanable<XdsClient>(
       work_serializer(), interested_parties_, StringView(server_name_.get()),
-      grpc_core::MakeUnique<ServiceConfigWatcher>(Ref()), *args_, &error);
+      absl::make_unique<ServiceConfigWatcher>(Ref()), *args_, &error);
   if (error != GRPC_ERROR_NONE) {
     gpr_log(GPR_ERROR,
             "Failed to create xds client -- channel will remain in "
@@ -130,7 +130,7 @@ class XdsResolverFactory : public ResolverFactory {
 
 void grpc_resolver_xds_init() {
   grpc_core::ResolverRegistry::Builder::RegisterResolverFactory(
-      grpc_core::MakeUnique<grpc_core::XdsResolverFactory>());
+      absl::make_unique<grpc_core::XdsResolverFactory>());
 }
 
 void grpc_resolver_xds_shutdown() {}

src/core/ext/filters/client_channel/resolver_result_parsing.cc

@@ -54,7 +54,7 @@ size_t ClientChannelServiceConfigParser::ParserIndex() {
 
 void ClientChannelServiceConfigParser::Register() {
   g_client_channel_service_config_parser_index = ServiceConfig::RegisterParser(
-      grpc_core::MakeUnique<ClientChannelServiceConfigParser>());
+      absl::make_unique<ClientChannelServiceConfigParser>());
 }
 
 namespace {
@@ -95,7 +95,7 @@ std::unique_ptr<ClientChannelMethodParsedConfig::RetryPolicy> ParseRetryPolicy(
     const Json& json, grpc_error** error) {
   GPR_DEBUG_ASSERT(error != nullptr && *error == GRPC_ERROR_NONE);
   auto retry_policy =
-      grpc_core::MakeUnique<ClientChannelMethodParsedConfig::RetryPolicy>();
+      absl::make_unique<ClientChannelMethodParsedConfig::RetryPolicy>();
   if (json.type() != Json::Type::OBJECT) {
     *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING(
         "field:retryPolicy error:should be of type object");
@@ -387,7 +387,7 @@ ClientChannelServiceConfigParser::ParseGlobalParams(const Json& json,
   *error = GRPC_ERROR_CREATE_FROM_VECTOR("Client channel global parser",
                                          &error_list);
   if (*error == GRPC_ERROR_NONE) {
-    return grpc_core::MakeUnique<ClientChannelGlobalParsedConfig>(
+    return absl::make_unique<ClientChannelGlobalParsedConfig>(
         std::move(parsed_lb_config), std::move(lb_policy_name),
         retry_throttling, health_check_service_name);
   }
@@ -433,7 +433,7 @@ ClientChannelServiceConfigParser::ParsePerMethodParams(const Json& json,
   }
   *error = GRPC_ERROR_CREATE_FROM_VECTOR("Client channel parser", &error_list);
   if (*error == GRPC_ERROR_NONE) {
-    return grpc_core::MakeUnique<ClientChannelMethodParsedConfig>(
+    return absl::make_unique<ClientChannelMethodParsedConfig>(
         timeout, wait_for_ready, std::move(retry_policy));
   }
   return nullptr;

src/core/ext/filters/client_channel/resolving_lb_policy.cc

@@ -187,15 +187,15 @@ ResolvingLoadBalancingPolicy::ResolvingLoadBalancingPolicy(
   GPR_ASSERT(process_resolver_result != nullptr);
   resolver_ = ResolverRegistry::CreateResolver(
       target_uri_.get(), args.args, interested_parties(), work_serializer(),
-      grpc_core::MakeUnique<ResolverResultHandler>(Ref()));
+      absl::make_unique<ResolverResultHandler>(Ref()));
   // Since the validity of args has been checked when create the channel,
   // CreateResolver() must return a non-null result.
   GPR_ASSERT(resolver_ != nullptr);
   if (GRPC_TRACE_FLAG_ENABLED(*tracer_)) {
     gpr_log(GPR_INFO, "resolving_lb=%p: starting name resolution", this);
   }
-  channel_control_helper()->UpdateState(
-      GRPC_CHANNEL_CONNECTING, grpc_core::MakeUnique<QueuePicker>(Ref()));
+  channel_control_helper()->UpdateState(GRPC_CHANNEL_CONNECTING,
+                                        absl::make_unique<QueuePicker>(Ref()));
   resolver_->StartLocked();
 }
 
@@ -261,7 +261,7 @@ void ResolvingLoadBalancingPolicy::OnResolverError(grpc_error* error) {
         "Resolver transient failure", &error, 1);
     channel_control_helper()->UpdateState(
         GRPC_CHANNEL_TRANSIENT_FAILURE,
-        grpc_core::MakeUnique<TransientFailurePicker>(state_error));
+        absl::make_unique<TransientFailurePicker>(state_error));
   }
   GRPC_ERROR_UNREF(error);
 }

src/core/ext/filters/client_channel/service_config.cc

@@ -95,7 +95,7 @@ grpc_error* ServiceConfig::ParseJsonMethodConfigToServiceConfigVectorTable(
     const Json& json,
     InlinedVector<SliceHashTable<const ParsedConfigVector*>::Entry, 10>*
         entries) {
-  auto objs_vector = grpc_core::MakeUnique<ParsedConfigVector>();
+  auto objs_vector = absl::make_unique<ParsedConfigVector>();
   InlinedVector<grpc_error*, 4> error_list;
   for (size_t i = 0; i < g_registered_parsers->size(); i++) {
     grpc_error* parser_error = GRPC_ERROR_NONE;

src/core/ext/filters/client_channel/xds/xds_api.cc

@@ -185,7 +185,8 @@ void PopulateMetadataValue(upb_arena* arena, google_protobuf_Value* value_pb,
 }
 
 void PopulateNode(upb_arena* arena, const XdsBootstrap::Node* node,
-                  const char* build_version, envoy_api_v2_core_Node* node_msg) {
+                  const char* build_version, const std::string& server_name,
+                  envoy_api_v2_core_Node* node_msg) {
   if (node != nullptr) {
     if (!node->id.empty()) {
       envoy_api_v2_core_Node_set_id(node_msg,
@@ -200,6 +201,18 @@ void PopulateNode(upb_arena* arena, const XdsBootstrap::Node* node,
           envoy_api_v2_core_Node_mutable_metadata(node_msg, arena);
       PopulateMetadata(arena, metadata, node->metadata.object_value());
     }
+    if (!server_name.empty()) {
+      google_protobuf_Struct* metadata =
+          envoy_api_v2_core_Node_mutable_metadata(node_msg, arena);
+      google_protobuf_Struct_FieldsEntry* field =
+          google_protobuf_Struct_add_fields(metadata, arena);
+      google_protobuf_Struct_FieldsEntry_set_key(
+          field, upb_strview_makez("PROXYLESS_CLIENT_HOSTNAME"));
+      google_protobuf_Value* value =
+          google_protobuf_Struct_FieldsEntry_mutable_value(field, arena);
+      google_protobuf_Value_set_string_value(
+          value, upb_strview_make(server_name.data(), server_name.size()));
+    }
     if (!node->locality_region.empty() || !node->locality_zone.empty() ||
         !node->locality_subzone.empty()) {
       envoy_api_v2_core_Locality* locality =
@@ -260,7 +273,7 @@ envoy_api_v2_DiscoveryRequest* CreateDiscoveryRequest(
   if (build_version != nullptr) {
     envoy_api_v2_core_Node* node_msg =
         envoy_api_v2_DiscoveryRequest_mutable_node(request, arena);
-    PopulateNode(arena, node, build_version, node_msg);
+    PopulateNode(arena, node, build_version, "", node_msg);
   }
   return request;
 }
@@ -960,33 +973,33 @@ grpc_slice XdsApi::CreateLrsInitialRequest(const std::string& server_name) {
   envoy_api_v2_core_Node* node_msg =
       envoy_service_load_stats_v2_LoadStatsRequest_mutable_node(request,
                                                                 arena.ptr());
-  PopulateNode(arena.ptr(), node_, build_version_, node_msg);
-  // Add cluster stats. There is only one because we only use one server name in
-  // one channel.
-  envoy_api_v2_endpoint_ClusterStats* cluster_stats =
-      envoy_service_load_stats_v2_LoadStatsRequest_add_cluster_stats(
-          request, arena.ptr());
-  // Set the cluster name.
-  envoy_api_v2_endpoint_ClusterStats_set_cluster_name(
-      cluster_stats, upb_strview_makez(server_name.c_str()));
+  PopulateNode(arena.ptr(), node_, build_version_, server_name, node_msg);
   return SerializeLrsRequest(request, arena.ptr());
 }
 
 namespace {
 
-void LocalityStatsPopulate(
-    envoy_api_v2_endpoint_UpstreamLocalityStats* output,
-    const std::pair<const RefCountedPtr<XdsLocalityName>,
-                    XdsClientStats::LocalityStats::Snapshot>& input,
-    upb_arena* arena) {
-  // Set sub_zone.
+void LocalityStatsPopulate(envoy_api_v2_endpoint_UpstreamLocalityStats* output,
+                           const XdsLocalityName& locality_name,
+                           const XdsClusterLocalityStats::Snapshot& snapshot,
+                           upb_arena* arena) {
+  // Set locality.
   envoy_api_v2_core_Locality* locality =
       envoy_api_v2_endpoint_UpstreamLocalityStats_mutable_locality(output,
                                                                    arena);
-  envoy_api_v2_core_Locality_set_sub_zone(
-      locality, upb_strview_makez(input.first->sub_zone().c_str()));
+  if (!locality_name.region().empty()) {
+    envoy_api_v2_core_Locality_set_region(
+        locality, upb_strview_makez(locality_name.region().c_str()));
+  }
+  if (!locality_name.zone().empty()) {
+    envoy_api_v2_core_Locality_set_zone(
+        locality, upb_strview_makez(locality_name.zone().c_str()));
+  }
+  if (!locality_name.sub_zone().empty()) {
+    envoy_api_v2_core_Locality_set_sub_zone(
+        locality, upb_strview_makez(locality_name.sub_zone().c_str()));
+  }
   // Set total counts.
-  const XdsClientStats::LocalityStats::Snapshot& snapshot = input.second;
   envoy_api_v2_endpoint_UpstreamLocalityStats_set_total_successful_requests(
       output, snapshot.total_successful_requests);
   envoy_api_v2_endpoint_UpstreamLocalityStats_set_total_requests_in_progress(
@@ -995,16 +1008,15 @@ void LocalityStatsPopulate(
       output, snapshot.total_error_requests);
   envoy_api_v2_endpoint_UpstreamLocalityStats_set_total_issued_requests(
       output, snapshot.total_issued_requests);
-  // Add load metric stats.
-  for (auto& p : snapshot.load_metric_stats) {
-    const char* metric_name = p.first.c_str();
-    const XdsClientStats::LocalityStats::LoadMetric::Snapshot& metric_value =
-        p.second;
+  // Add backend metrics.
+  for (const auto& p : snapshot.backend_metrics) {
+    const std::string& metric_name = p.first;
+    const XdsClusterLocalityStats::BackendMetric& metric_value = p.second;
     envoy_api_v2_endpoint_EndpointLoadMetricStats* load_metric =
         envoy_api_v2_endpoint_UpstreamLocalityStats_add_load_metric_stats(
             output, arena);
     envoy_api_v2_endpoint_EndpointLoadMetricStats_set_metric_name(
-        load_metric, upb_strview_makez(metric_name));
+        load_metric, upb_strview_make(metric_name.data(), metric_name.size()));
     envoy_api_v2_endpoint_EndpointLoadMetricStats_set_num_requests_finished_with_metric(
         load_metric, metric_value.num_requests_finished_with_metric);
     envoy_api_v2_endpoint_EndpointLoadMetricStats_set_total_metric_value(
@@ -1015,74 +1027,64 @@ void LocalityStatsPopulate(
 }  // namespace
 
 grpc_slice XdsApi::CreateLrsRequest(
-    std::map<StringView, std::set<XdsClientStats*>, StringLess>
-        client_stats_map) {
+    ClusterLoadReportMap cluster_load_report_map) {
   upb::Arena arena;
-  // Get the snapshots.
-  std::map<StringView, grpc_core::InlinedVector<XdsClientStats::Snapshot, 1>,
-           StringLess>
-      snapshot_map;
-  for (auto& p : client_stats_map) {
-    const StringView& cluster_name = p.first;
-    for (auto* client_stats : p.second) {
-      XdsClientStats::Snapshot snapshot = client_stats->GetSnapshotAndReset();
-      // Prune unused locality stats.
-      client_stats->PruneLocalityStats();
-      if (snapshot.IsAllZero()) continue;
-      snapshot_map[cluster_name].emplace_back(std::move(snapshot));
-    }
-  }
-  // When all the counts are zero, return empty slice.
-  if (snapshot_map.empty()) return grpc_empty_slice();
   // Create a request.
   envoy_service_load_stats_v2_LoadStatsRequest* request =
       envoy_service_load_stats_v2_LoadStatsRequest_new(arena.ptr());
-  for (auto& p : snapshot_map) {
-    const StringView& cluster_name = p.first;
-    const auto& snapshot_list = p.second;
-    for (size_t i = 0; i < snapshot_list.size(); ++i) {
-      const auto& snapshot = snapshot_list[i];
-      // Add cluster stats.
-      envoy_api_v2_endpoint_ClusterStats* cluster_stats =
-          envoy_service_load_stats_v2_LoadStatsRequest_add_cluster_stats(
-              request, arena.ptr());
-      // Set the cluster name.
-      envoy_api_v2_endpoint_ClusterStats_set_cluster_name(
+  for (auto& p : cluster_load_report_map) {
+    const std::string& cluster_name = p.first.first;
+    const std::string& eds_service_name = p.first.second;
+    const ClusterLoadReport& load_report = p.second;
+    // Add cluster stats.
+    envoy_api_v2_endpoint_ClusterStats* cluster_stats =
+        envoy_service_load_stats_v2_LoadStatsRequest_add_cluster_stats(
+            request, arena.ptr());
+    // Set the cluster name.
+    envoy_api_v2_endpoint_ClusterStats_set_cluster_name(
+        cluster_stats,
+        upb_strview_make(cluster_name.data(), cluster_name.size()));
+    // Set EDS service name, if non-empty.
+    if (!eds_service_name.empty()) {
+      envoy_api_v2_endpoint_ClusterStats_set_cluster_service_name(
           cluster_stats,
-          upb_strview_make(cluster_name.data(), cluster_name.size()));
-      // Add locality stats.
-      for (auto& p : snapshot.upstream_locality_stats) {
-        envoy_api_v2_endpoint_UpstreamLocalityStats* locality_stats =
-            envoy_api_v2_endpoint_ClusterStats_add_upstream_locality_stats(
-                cluster_stats, arena.ptr());
-        LocalityStatsPopulate(locality_stats, p, arena.ptr());
-      }
-      // Add dropped requests.
-      for (auto& p : snapshot.dropped_requests) {
-        const char* category = p.first.c_str();
-        const uint64_t count = p.second;
-        envoy_api_v2_endpoint_ClusterStats_DroppedRequests* dropped_requests =
-            envoy_api_v2_endpoint_ClusterStats_add_dropped_requests(
-                cluster_stats, arena.ptr());
-        envoy_api_v2_endpoint_ClusterStats_DroppedRequests_set_category(
-            dropped_requests, upb_strview_makez(category));
-        envoy_api_v2_endpoint_ClusterStats_DroppedRequests_set_dropped_count(
-            dropped_requests, count);
-      }
-      // Set total dropped requests.
-      envoy_api_v2_endpoint_ClusterStats_set_total_dropped_requests(
-          cluster_stats, snapshot.total_dropped_requests);
-      // Set real load report interval.
-      gpr_timespec timespec =
-          grpc_millis_to_timespec(snapshot.load_report_interval, GPR_TIMESPAN);
-      google_protobuf_Duration* load_report_interval =
-          envoy_api_v2_endpoint_ClusterStats_mutable_load_report_interval(
+          upb_strview_make(eds_service_name.data(), eds_service_name.size()));
+    }
+    // Add locality stats.
+    for (const auto& p : load_report.locality_stats) {
+      const XdsLocalityName& locality_name = *p.first;
+      const auto& snapshot = p.second;
+      envoy_api_v2_endpoint_UpstreamLocalityStats* locality_stats =
+          envoy_api_v2_endpoint_ClusterStats_add_upstream_locality_stats(
               cluster_stats, arena.ptr());
-      google_protobuf_Duration_set_seconds(load_report_interval,
-                                           timespec.tv_sec);
-      google_protobuf_Duration_set_nanos(load_report_interval,
-                                         timespec.tv_nsec);
+      LocalityStatsPopulate(locality_stats, locality_name, snapshot,
+                            arena.ptr());
+    }
+    // Add dropped requests.
+    uint64_t total_dropped_requests = 0;
+    for (const auto& p : load_report.dropped_requests) {
+      const char* category = p.first.c_str();
+      const uint64_t count = p.second;
+      envoy_api_v2_endpoint_ClusterStats_DroppedRequests* dropped_requests =
+          envoy_api_v2_endpoint_ClusterStats_add_dropped_requests(cluster_stats,
+                                                                  arena.ptr());
+      envoy_api_v2_endpoint_ClusterStats_DroppedRequests_set_category(
+          dropped_requests, upb_strview_makez(category));
+      envoy_api_v2_endpoint_ClusterStats_DroppedRequests_set_dropped_count(
+          dropped_requests, count);
+      total_dropped_requests += count;
     }
+    // Set total dropped requests.
+    envoy_api_v2_endpoint_ClusterStats_set_total_dropped_requests(
+        cluster_stats, total_dropped_requests);
+    // Set real load report interval.
+    gpr_timespec timespec =
+        grpc_millis_to_timespec(load_report.load_report_interval, GPR_TIMESPAN);
+    google_protobuf_Duration* load_report_interval =
+        envoy_api_v2_endpoint_ClusterStats_mutable_load_report_interval(
+            cluster_stats, arena.ptr());
+    google_protobuf_Duration_set_seconds(load_report_interval, timespec.tv_sec);
+    google_protobuf_Duration_set_nanos(load_report_interval, timespec.tv_nsec);
   }
   return SerializeLrsRequest(request, arena.ptr());
 }

src/core/ext/filters/client_channel/xds/xds_api.h

@@ -176,6 +176,17 @@ class XdsApi {
 
   using EdsUpdateMap = std::map<std::string /*eds_service_name*/, EdsUpdate>;
 
+  struct ClusterLoadReport {
+    XdsClusterDropStats::DroppedRequestsMap dropped_requests;
+    std::map<XdsLocalityName*, XdsClusterLocalityStats::Snapshot,
+             XdsLocalityName::Less>
+        locality_stats;
+    grpc_millis load_report_interval;
+  };
+  using ClusterLoadReportMap = std::map<
+      std::pair<std::string /*cluster_name*/, std::string /*eds_service_name*/>,
+      ClusterLoadReport>;
+
   XdsApi(const XdsBootstrap::Node* node, const char* build_version)
       : node_(node), build_version_(build_version) {}
 
@@ -228,11 +239,8 @@ class XdsApi {
   // Creates an LRS request querying \a server_name.
   grpc_slice CreateLrsInitialRequest(const std::string& server_name);
 
-  // Creates an LRS request sending client-side load reports. If all the
-  // counters are zero, returns empty slice.
-  grpc_slice CreateLrsRequest(std::map<StringView /*cluster_name*/,
-                                       std::set<XdsClientStats*>, StringLess>
-                                  client_stats_map);
+  // Creates an LRS request sending a client-side load report.
+  grpc_slice CreateLrsRequest(ClusterLoadReportMap cluster_load_report_map);
 
   // Parses the LRS response and returns \a
   // load_reporting_interval for client-side load reporting. If there is any

src/core/ext/filters/client_channel/xds/xds_bootstrap.cc

@@ -42,7 +42,7 @@ std::unique_ptr<XdsBootstrap> XdsBootstrap::ReadFromFile(grpc_error** error) {
   Json json = Json::Parse(StringViewFromSlice(contents), error);
   grpc_slice_unref_internal(contents);
   if (*error != GRPC_ERROR_NONE) return nullptr;
-  return grpc_core::MakeUnique<XdsBootstrap>(std::move(json), error);
+  return absl::make_unique<XdsBootstrap>(std::move(json), error);
 }
 
 XdsBootstrap::XdsBootstrap(Json json, grpc_error** error) {
@@ -192,7 +192,7 @@ grpc_error* XdsBootstrap::ParseChannelCreds(Json* json, size_t idx,
 
 grpc_error* XdsBootstrap::ParseNode(Json* json) {
   InlinedVector<grpc_error*, 1> error_list;
-  node_ = grpc_core::MakeUnique<Node>();
+  node_ = absl::make_unique<Node>();
   auto it = json->mutable_object()->find("id");
   if (it != json->mutable_object()->end()) {
     if (it->second.type() != Json::Type::STRING) {

src/core/ext/filters/client_channel/xds/xds_client.cc

@@ -1305,19 +1305,39 @@ void XdsClient::ChannelState::LrsCallState::Reporter::OnNextReportTimerLocked(
   GRPC_ERROR_UNREF(error);
 }
 
+namespace {
+
+bool LoadReportCountersAreZero(const XdsApi::ClusterLoadReportMap& snapshot) {
+  for (const auto& p : snapshot) {
+    const XdsApi::ClusterLoadReport& cluster_snapshot = p.second;
+    for (const auto& q : cluster_snapshot.dropped_requests) {
+      if (q.second > 0) return false;
+    }
+    for (const auto& q : cluster_snapshot.locality_stats) {
+      const XdsClusterLocalityStats::Snapshot& locality_snapshot = q.second;
+      if (!locality_snapshot.IsZero()) return false;
+    }
+  }
+  return true;
+}
+
+}  // namespace
+
 void XdsClient::ChannelState::LrsCallState::Reporter::SendReportLocked() {
-  // Create a request that contains the load report.
-  grpc_slice request_payload_slice =
-      xds_client()->api_.CreateLrsRequest(xds_client()->ClientStatsMap());
+  // Construct snapshot from all reported stats.
+  XdsApi::ClusterLoadReportMap snapshot =
+      xds_client()->BuildLoadReportSnapshot();
   // Skip client load report if the counters were all zero in the last
   // report and they are still zero in this one.
   const bool old_val = last_report_counters_were_zero_;
-  last_report_counters_were_zero_ = static_cast<bool>(
-      grpc_slice_eq(request_payload_slice, grpc_empty_slice()));
+  last_report_counters_were_zero_ = LoadReportCountersAreZero(snapshot);
   if (old_val && last_report_counters_were_zero_) {
     ScheduleNextReportLocked();
     return;
   }
+  // Create a request that contains the snapshot.
+  grpc_slice request_payload_slice =
+      xds_client()->api_.CreateLrsRequest(std::move(snapshot));
   parent_->send_message_payload_ =
       grpc_raw_byte_buffer_create(&request_payload_slice, 1);
   grpc_slice_unref_internal(request_payload_slice);
@@ -1496,11 +1516,6 @@ void XdsClient::ChannelState::LrsCallState::MaybeStartReportingLocked() {
   AdsCallState* ads_calld = chand()->ads_calld_->calld();
   if (ads_calld == nullptr || !ads_calld->seen_response()) return;
   // Start reporting.
-  for (auto& p : chand()->xds_client_->endpoint_map_) {
-    for (auto* client_stats : p.second.client_stats) {
-      client_stats->MaybeInitLastReportTime();
-    }
-  }
   reporter_ = MakeOrphanable<Reporter>(
       Ref(DEBUG_LOCATION, "LRS+load_report+start"), load_reporting_interval_);
 }
@@ -1797,31 +1812,104 @@ void XdsClient::CancelEndpointDataWatch(StringView eds_service_name,
   }
 }
 
-void XdsClient::AddClientStats(StringView /*lrs_server*/,
-                               StringView cluster_name,
-                               XdsClientStats* client_stats) {
-  EndpointState& endpoint_state = endpoint_map_[std::string(cluster_name)];
+RefCountedPtr<XdsClusterDropStats> XdsClient::AddClusterDropStats(
+    StringView lrs_server, StringView cluster_name,
+    StringView eds_service_name) {
   // TODO(roth): When we add support for direct federation, use the
   // server name specified in lrs_server.
-  endpoint_state.client_stats.insert(client_stats);
+  auto key =
+      std::make_pair(std::string(cluster_name), std::string(eds_service_name));
+  // We jump through some hoops here to make sure that the StringViews
+  // stored in the XdsClusterDropStats object point to the strings
+  // in the load_report_map_ key, so that they have the same lifetime.
+  auto it = load_report_map_
+                .emplace(std::make_pair(std::move(key), LoadReportState()))
+                .first;
+  auto cluster_drop_stats = MakeRefCounted<XdsClusterDropStats>(
+      Ref(DEBUG_LOCATION, "DropStats"), lrs_server,
+      it->first.first /*cluster_name*/, it->first.second /*eds_service_name*/);
+  it->second.drop_stats.insert(cluster_drop_stats.get());
   chand_->MaybeStartLrsCall();
+  return cluster_drop_stats;
 }
 
-void XdsClient::RemoveClientStats(StringView /*lrs_server*/,
-                                  StringView cluster_name,
-                                  XdsClientStats* client_stats) {
-  EndpointState& endpoint_state = endpoint_map_[std::string(cluster_name)];
+void XdsClient::RemoveClusterDropStats(
+    StringView /*lrs_server*/, StringView cluster_name,
+    StringView eds_service_name, XdsClusterDropStats* cluster_drop_stats) {
+  auto load_report_it = load_report_map_.find(
+      std::make_pair(std::string(cluster_name), std::string(eds_service_name)));
+  if (load_report_it == load_report_map_.end()) return;
+  LoadReportState& load_report_state = load_report_it->second;
   // TODO(roth): When we add support for direct federation, use the
   // server name specified in lrs_server.
   // TODO(roth): In principle, we should try to send a final load report
   // containing whatever final stats have been accumulated since the
   // last load report.
-  auto it = endpoint_state.client_stats.find(client_stats);
-  if (it != endpoint_state.client_stats.end()) {
-    endpoint_state.client_stats.erase(it);
+  auto it = load_report_state.drop_stats.find(cluster_drop_stats);
+  if (it != load_report_state.drop_stats.end()) {
+    load_report_state.drop_stats.erase(it);
+    if (load_report_state.drop_stats.empty() &&
+        load_report_state.locality_stats.empty()) {
+      load_report_map_.erase(load_report_it);
+      if (chand_ != nullptr && load_report_map_.empty()) {
+        chand_->StopLrsCall();
+      }
+    }
   }
-  if (chand_ != nullptr && endpoint_state.client_stats.empty()) {
-    chand_->StopLrsCall();
+}
+
+RefCountedPtr<XdsClusterLocalityStats> XdsClient::AddClusterLocalityStats(
+    StringView lrs_server, StringView cluster_name, StringView eds_service_name,
+    RefCountedPtr<XdsLocalityName> locality) {
+  // TODO(roth): When we add support for direct federation, use the
+  // server name specified in lrs_server.
+  auto key =
+      std::make_pair(std::string(cluster_name), std::string(eds_service_name));
+  // We jump through some hoops here to make sure that the StringViews
+  // stored in the XdsClusterLocalityStats object point to the strings
+  // in the load_report_map_ key, so that they have the same lifetime.
+  auto it = load_report_map_
+                .emplace(std::make_pair(std::move(key), LoadReportState()))
+                .first;
+  auto cluster_locality_stats = MakeRefCounted<XdsClusterLocalityStats>(
+      Ref(DEBUG_LOCATION, "LocalityStats"), lrs_server,
+      it->first.first /*cluster_name*/, it->first.second /*eds_service_name*/,
+      locality);
+  it->second.locality_stats[std::move(locality)].insert(
+      cluster_locality_stats.get());
+  chand_->MaybeStartLrsCall();
+  return cluster_locality_stats;
+}
+
+void XdsClient::RemoveClusterLocalityStats(
+    StringView /*lrs_server*/, StringView cluster_name,
+    StringView eds_service_name, const RefCountedPtr<XdsLocalityName>& locality,
+    XdsClusterLocalityStats* cluster_locality_stats) {
+  auto load_report_it = load_report_map_.find(
+      std::make_pair(std::string(cluster_name), std::string(eds_service_name)));
+  if (load_report_it == load_report_map_.end()) return;
+  LoadReportState& load_report_state = load_report_it->second;
+  // TODO(roth): When we add support for direct federation, use the
+  // server name specified in lrs_server.
+  // TODO(roth): In principle, we should try to send a final load report
+  // containing whatever final stats have been accumulated since the
+  // last load report.
+  auto locality_it = load_report_state.locality_stats.find(locality);
+  if (locality_it == load_report_state.locality_stats.end()) return;
+  auto& locality_set = locality_it->second;
+  auto it = locality_set.find(cluster_locality_stats);
+  if (it != locality_set.end()) {
+    locality_set.erase(it);
+    if (locality_set.empty()) {
+      load_report_state.locality_stats.erase(locality_it);
+      if (load_report_state.locality_stats.empty() &&
+          load_report_state.drop_stats.empty()) {
+        load_report_map_.erase(load_report_it);
+        if (chand_ != nullptr && load_report_map_.empty()) {
+          chand_->StopLrsCall();
+        }
+      }
+    }
   }
 }
 
@@ -1850,17 +1938,34 @@ grpc_error* XdsClient::CreateServiceConfig(
   return error;
 }
 
-std::map<StringView, std::set<XdsClientStats*>, StringLess>
-XdsClient::ClientStatsMap() const {
-  std::map<StringView, std::set<XdsClientStats*>, StringLess> client_stats_map;
-  for (const auto& p : endpoint_map_) {
-    const StringView cluster_name = p.first;
-    const auto& client_stats = p.second.client_stats;
-    if (chand_->lrs_calld()->ShouldSendLoadReports(cluster_name)) {
-      client_stats_map.emplace(cluster_name, client_stats);
+XdsApi::ClusterLoadReportMap XdsClient::BuildLoadReportSnapshot() {
+  XdsApi::ClusterLoadReportMap snapshot_map;
+  for (auto& p : load_report_map_) {
+    const auto& cluster_key = p.first;  // cluster and EDS service name
+    LoadReportState& load_report = p.second;
+    XdsApi::ClusterLoadReport& snapshot = snapshot_map[cluster_key];
+    // Aggregate drop stats.
+    for (auto& drop_stats : load_report.drop_stats) {
+      for (const auto& p : drop_stats->GetSnapshotAndReset()) {
+        snapshot.dropped_requests[p.first] += p.second;
+      }
+    }
+    // Aggregate locality stats.
+    for (auto& p : load_report.locality_stats) {
+      XdsLocalityName* locality_name = p.first.get();
+      auto& locality_stats_set = p.second;
+      XdsClusterLocalityStats::Snapshot& locality_snapshot =
+          snapshot.locality_stats[locality_name];
+      for (auto& locality_stats : locality_stats_set) {
+        locality_snapshot += locality_stats->GetSnapshotAndReset();
+      }
     }
+    // Compute load report interval.
+    const grpc_millis now = ExecCtx::Get()->Now();
+    snapshot.load_report_interval = now - load_report.last_report_time;
+    load_report.last_report_time = now;
   }
-  return client_stats_map;
+  return snapshot_map;
 }
 
 void XdsClient::NotifyOnError(grpc_error* error) {

src/core/ext/filters/client_channel/xds/xds_client.h

@@ -100,11 +100,25 @@ class XdsClient : public InternallyRefCounted<XdsClient> {
   void CancelEndpointDataWatch(StringView eds_service_name,
                                EndpointWatcherInterface* watcher);
 
-  // Adds and removes client stats for \a cluster_name.
-  void AddClientStats(StringView /*lrs_server*/, StringView cluster_name,
-                      XdsClientStats* client_stats);
-  void RemoveClientStats(StringView /*lrs_server*/, StringView cluster_name,
-                         XdsClientStats* client_stats);
+  // Adds and removes drop stats for cluster_name and eds_service_name.
+  RefCountedPtr<XdsClusterDropStats> AddClusterDropStats(
+      StringView lrs_server, StringView cluster_name,
+      StringView eds_service_name);
+  void RemoveClusterDropStats(StringView /*lrs_server*/,
+                              StringView cluster_name,
+                              StringView eds_service_name,
+                              XdsClusterDropStats* cluster_drop_stats);
+
+  // Adds and removes locality stats for cluster_name and eds_service_name
+  // for the specified locality.
+  RefCountedPtr<XdsClusterLocalityStats> AddClusterLocalityStats(
+      StringView lrs_server, StringView cluster_name,
+      StringView eds_service_name, RefCountedPtr<XdsLocalityName> locality);
+  void RemoveClusterLocalityStats(
+      StringView /*lrs_server*/, StringView cluster_name,
+      StringView eds_service_name,
+      const RefCountedPtr<XdsLocalityName>& locality,
+      XdsClusterLocalityStats* cluster_locality_stats);
 
   // Resets connection backoff state.
   void ResetBackoff();
@@ -181,11 +195,18 @@ class XdsClient : public InternallyRefCounted<XdsClient> {
     std::map<EndpointWatcherInterface*,
              std::unique_ptr<EndpointWatcherInterface>>
         watchers;
-    std::set<XdsClientStats*> client_stats;
     // The latest data seen from EDS.
     XdsApi::EdsUpdate update;
   };
 
+  struct LoadReportState {
+    std::set<XdsClusterDropStats*> drop_stats;
+    std::map<RefCountedPtr<XdsLocalityName>, std::set<XdsClusterLocalityStats*>,
+             XdsLocalityName::Less>
+        locality_stats;
+    grpc_millis last_report_time = ExecCtx::Get()->Now();
+  };
+
   // Sends an error notification to all watchers.
   void NotifyOnError(grpc_error* error);
 
@@ -193,8 +214,7 @@ class XdsClient : public InternallyRefCounted<XdsClient> {
       const std::string& cluster_name,
       RefCountedPtr<ServiceConfig>* service_config) const;
 
-  std::map<StringView, std::set<XdsClientStats*>, StringLess> ClientStatsMap()
-      const;
+  XdsApi::ClusterLoadReportMap BuildLoadReportSnapshot();
 
   // Channel arg vtable functions.
   static void* ChannelArgCopy(void* p);
@@ -226,6 +246,10 @@ class XdsClient : public InternallyRefCounted<XdsClient> {
   std::map<std::string /*cluster_name*/, ClusterState> cluster_map_;
   // Only the watched EDS service names are stored.
   std::map<std::string /*eds_service_name*/, EndpointState> endpoint_map_;
+  std::map<
+      std::pair<std::string /*cluster_name*/, std::string /*eds_service_name*/>,
+      LoadReportState>
+      load_report_map_;
 
   bool shutting_down_ = false;
 };

src/core/ext/filters/client_channel/xds/xds_client_stats.cc

@@ -20,63 +20,75 @@
 
 #include "src/core/ext/filters/client_channel/xds/xds_client_stats.h"
 
+#include <string.h>
+
 #include <grpc/support/atm.h>
 #include <grpc/support/string_util.h>
-#include <string.h>
+
+#include "src/core/ext/filters/client_channel/xds/xds_client.h"
 
 namespace grpc_core {
 
-namespace {
+//
+// XdsClusterDropStats
+//
 
-template <typename T>
-T GetAndResetCounter(Atomic<T>* from) {
-  return from->Exchange(0, MemoryOrder::RELAXED);
+XdsClusterDropStats::XdsClusterDropStats(RefCountedPtr<XdsClient> xds_client,
+                                         StringView lrs_server_name,
+                                         StringView cluster_name,
+                                         StringView eds_service_name)
+    : xds_client_(std::move(xds_client)),
+      lrs_server_name_(lrs_server_name),
+      cluster_name_(cluster_name),
+      eds_service_name_(eds_service_name) {}
+
+XdsClusterDropStats::~XdsClusterDropStats() {
+  xds_client_->RemoveClusterDropStats(lrs_server_name_, cluster_name_,
+                                      eds_service_name_, this);
+  xds_client_.reset(DEBUG_LOCATION, "DropStats");
 }
 
-}  // namespace
-
-//
-// XdsClientStats::LocalityStats::LoadMetric::Snapshot
-//
+XdsClusterDropStats::DroppedRequestsMap
+XdsClusterDropStats::GetSnapshotAndReset() {
+  MutexLock lock(&mu_);
+  return std::move(dropped_requests_);
+}
 
-bool XdsClientStats::LocalityStats::LoadMetric::Snapshot::IsAllZero() const {
-  return total_metric_value == 0 && num_requests_finished_with_metric == 0;
+void XdsClusterDropStats::AddCallDropped(const std::string& category) {
+  MutexLock lock(&mu_);
+  ++dropped_requests_[category];
 }
 
 //
-// XdsClientStats::LocalityStats::LoadMetric
+// XdsClusterLocalityStats
 //
 
-XdsClientStats::LocalityStats::LoadMetric::Snapshot
-XdsClientStats::LocalityStats::LoadMetric::GetSnapshotAndReset() {
-  Snapshot metric = {num_requests_finished_with_metric_, total_metric_value_};
-  num_requests_finished_with_metric_ = 0;
-  total_metric_value_ = 0;
-  return metric;
+XdsClusterLocalityStats::XdsClusterLocalityStats(
+    RefCountedPtr<XdsClient> xds_client, StringView lrs_server_name,
+    StringView cluster_name, StringView eds_service_name,
+    RefCountedPtr<XdsLocalityName> name)
+    : xds_client_(std::move(xds_client)),
+      lrs_server_name_(lrs_server_name),
+      cluster_name_(cluster_name),
+      eds_service_name_(eds_service_name),
+      name_(std::move(name)) {}
+
+XdsClusterLocalityStats::~XdsClusterLocalityStats() {
+  xds_client_->RemoveClusterLocalityStats(lrs_server_name_, cluster_name_,
+                                          eds_service_name_, name_, this);
+  xds_client_.reset(DEBUG_LOCATION, "LocalityStats");
 }
 
-//
-// XdsClientStats::LocalityStats::Snapshot
-//
+namespace {
 
-bool XdsClientStats::LocalityStats::Snapshot::IsAllZero() {
-  if (total_successful_requests != 0 || total_requests_in_progress != 0 ||
-      total_error_requests != 0 || total_issued_requests != 0) {
-    return false;
-  }
-  for (auto& p : load_metric_stats) {
-    const LoadMetric::Snapshot& metric_value = p.second;
-    if (!metric_value.IsAllZero()) return false;
-  }
-  return true;
+uint64_t GetAndResetCounter(Atomic<uint64_t>* from) {
+  return from->Exchange(0, MemoryOrder::RELAXED);
 }
 
-//
-// XdsClientStats::LocalityStats
-//
+}  // namespace
 
-XdsClientStats::LocalityStats::Snapshot
-XdsClientStats::LocalityStats::GetSnapshotAndReset() {
+XdsClusterLocalityStats::Snapshot
+XdsClusterLocalityStats::GetSnapshotAndReset() {
   Snapshot snapshot = {
       GetAndResetCounter(&total_successful_requests_),
       // Don't reset total_requests_in_progress because it's not
@@ -84,108 +96,21 @@ XdsClientStats::LocalityStats::GetSnapshotAndReset() {
       total_requests_in_progress_.Load(MemoryOrder::RELAXED),
       GetAndResetCounter(&total_error_requests_),
       GetAndResetCounter(&total_issued_requests_)};
-  {
-    MutexLock lock(&load_metric_stats_mu_);
-    for (auto& p : load_metric_stats_) {
-      const std::string& metric_name = p.first;
-      LoadMetric& metric_value = p.second;
-      snapshot.load_metric_stats.emplace(metric_name,
-                                         metric_value.GetSnapshotAndReset());
-    }
-  }
+  MutexLock lock(&backend_metrics_mu_);
+  snapshot.backend_metrics = std::move(backend_metrics_);
   return snapshot;
 }
 
-void XdsClientStats::LocalityStats::AddCallStarted() {
+void XdsClusterLocalityStats::AddCallStarted() {
   total_issued_requests_.FetchAdd(1, MemoryOrder::RELAXED);
   total_requests_in_progress_.FetchAdd(1, MemoryOrder::RELAXED);
 }
 
-void XdsClientStats::LocalityStats::AddCallFinished(bool fail) {
+void XdsClusterLocalityStats::AddCallFinished(bool fail) {
   Atomic<uint64_t>& to_increment =
       fail ? total_error_requests_ : total_successful_requests_;
   to_increment.FetchAdd(1, MemoryOrder::RELAXED);
   total_requests_in_progress_.FetchAdd(-1, MemoryOrder::ACQ_REL);
 }
 
-//
-// XdsClientStats::Snapshot
-//
-
-bool XdsClientStats::Snapshot::IsAllZero() {
-  for (auto& p : upstream_locality_stats) {
-    if (!p.second.IsAllZero()) return false;
-  }
-  for (auto& p : dropped_requests) {
-    if (p.second != 0) return false;
-  }
-  return total_dropped_requests == 0;
-}
-
-//
-// XdsClientStats
-//
-
-XdsClientStats::Snapshot XdsClientStats::GetSnapshotAndReset() {
-  grpc_millis now = ExecCtx::Get()->Now();
-  // Record total_dropped_requests and reporting interval in the snapshot.
-  Snapshot snapshot;
-  snapshot.total_dropped_requests =
-      GetAndResetCounter(&total_dropped_requests_);
-  snapshot.load_report_interval = now - last_report_time_;
-  // Update last report time.
-  last_report_time_ = now;
-  // Snapshot all the other stats.
-  for (auto& p : upstream_locality_stats_) {
-    snapshot.upstream_locality_stats.emplace(p.first,
-                                             p.second->GetSnapshotAndReset());
-  }
-  {
-    MutexLock lock(&dropped_requests_mu_);
-    // This is a workaround for the case where some compilers cannot build
-    // move-assignment of map with non-copyable but movable key.
-    // https://stackoverflow.com/questions/36475497
-    std::swap(snapshot.dropped_requests, dropped_requests_);
-    dropped_requests_.clear();
-  }
-  return snapshot;
-}
-
-void XdsClientStats::MaybeInitLastReportTime() {
-  if (last_report_time_ == -1) last_report_time_ = ExecCtx::Get()->Now();
-}
-
-RefCountedPtr<XdsClientStats::LocalityStats> XdsClientStats::FindLocalityStats(
-    const RefCountedPtr<XdsLocalityName>& locality_name) {
-  auto iter = upstream_locality_stats_.find(locality_name);
-  if (iter == upstream_locality_stats_.end()) {
-    iter = upstream_locality_stats_
-               .emplace(locality_name, MakeRefCounted<LocalityStats>())
-               .first;
-  }
-  return iter->second;
-}
-
-void XdsClientStats::PruneLocalityStats() {
-  auto iter = upstream_locality_stats_.begin();
-  while (iter != upstream_locality_stats_.end()) {
-    if (iter->second->IsSafeToDelete()) {
-      iter = upstream_locality_stats_.erase(iter);
-    } else {
-      ++iter;
-    }
-  }
-}
-
-void XdsClientStats::AddCallDropped(const std::string& category) {
-  total_dropped_requests_.FetchAdd(1, MemoryOrder::RELAXED);
-  MutexLock lock(&dropped_requests_mu_);
-  auto iter = dropped_requests_.find(category);
-  if (iter == dropped_requests_.end()) {
-    dropped_requests_.emplace(category, 1);
-  } else {
-    ++iter->second;
-  }
-}
-
 }  // namespace grpc_core

src/core/ext/filters/client_channel/xds/xds_client_stats.h

@@ -33,17 +33,26 @@
 
 namespace grpc_core {
 
+// Forward declaration to avoid circular dependency.
+class XdsClient;
+
+// Locality name.
 class XdsLocalityName : public RefCounted<XdsLocalityName> {
  public:
   struct Less {
-    bool operator()(const RefCountedPtr<XdsLocalityName>& lhs,
-                    const RefCountedPtr<XdsLocalityName>& rhs) const {
+    bool operator()(const XdsLocalityName* lhs,
+                    const XdsLocalityName* rhs) const {
       int cmp_result = lhs->region_.compare(rhs->region_);
       if (cmp_result != 0) return cmp_result < 0;
       cmp_result = lhs->zone_.compare(rhs->zone_);
       if (cmp_result != 0) return cmp_result < 0;
       return lhs->sub_zone_.compare(rhs->sub_zone_) < 0;
     }
+
+    bool operator()(const RefCountedPtr<XdsLocalityName>& lhs,
+                    const RefCountedPtr<XdsLocalityName>& rhs) const {
+      return (*this)(lhs.get(), rhs.get());
+    }
   };
 
   XdsLocalityName(std::string region, std::string zone, std::string subzone)
@@ -77,150 +86,112 @@ class XdsLocalityName : public RefCounted<XdsLocalityName> {
   UniquePtr<char> human_readable_string_;
 };
 
-// The stats classes (i.e., XdsClientStats, LocalityStats, and LoadMetric) can
-// be taken a snapshot (and reset) to populate the load report. The snapshots
-// are contained in the respective Snapshot structs. The Snapshot structs have
-// no synchronization. The stats classes use several different synchronization
-// methods. 1. Most of the counters are Atomic<>s for performance. 2. Some of
-// the Map<>s are protected by Mutex if we are not guaranteed that the accesses
-// to them are synchronized by the callers. 3. The Map<>s to which the accesses
-// are already synchronized by the callers do not have additional
-// synchronization here. Note that the Map<>s we mentioned in 2 and 3 refer to
-// the map's tree structure rather than the content in each tree node.
-class XdsClientStats {
+// Drop stats for an xds cluster.
+class XdsClusterDropStats : public RefCounted<XdsClusterDropStats> {
  public:
-  class LocalityStats : public RefCounted<LocalityStats> {
-   public:
-    class LoadMetric {
-     public:
-      struct Snapshot {
-        bool IsAllZero() const;
-
-        uint64_t num_requests_finished_with_metric;
-        double total_metric_value;
-      };
-
-      // Returns a snapshot of this instance and reset all the accumulative
-      // counters.
-      Snapshot GetSnapshotAndReset();
-
-     private:
-      uint64_t num_requests_finished_with_metric_{0};
-      double total_metric_value_{0};
-    };
-
-    using LoadMetricMap = std::map<std::string, LoadMetric>;
-    using LoadMetricSnapshotMap = std::map<std::string, LoadMetric::Snapshot>;
-
-    struct Snapshot {
-      // TODO(juanlishen): Change this to const method when const_iterator is
-      // added to Map<>.
-      bool IsAllZero();
-
-      uint64_t total_successful_requests;
-      uint64_t total_requests_in_progress;
-      uint64_t total_error_requests;
-      uint64_t total_issued_requests;
-      LoadMetricSnapshotMap load_metric_stats;
-    };
-
-    // Returns a snapshot of this instance and reset all the accumulative
-    // counters.
-    Snapshot GetSnapshotAndReset();
-
-    // Each XdsLb::PickerWrapper holds a ref to the perspective LocalityStats.
-    // If the refcount is 0, there won't be new calls recorded to the
-    // LocalityStats, so the LocalityStats can be safely deleted when all the
-    // in-progress calls have finished.
-    // Only be called from the control plane work_serializer.
-    void RefByPicker() { picker_refcount_.FetchAdd(1, MemoryOrder::ACQ_REL); }
-    // Might be called from the control plane work_serializer or the data plane
-    // mutex.
-    // TODO(juanlishen): Once https://github.com/grpc/grpc/pull/19390 is merged,
-    //  this method will also only be invoked in the control plane
-    //  work_serializer. We may then be able to simplify the LocalityStats'
-    //  lifetime by making it RefCounted<> and populating the protobuf in its
-    //  dtor.
-    void UnrefByPicker() { picker_refcount_.FetchSub(1, MemoryOrder::ACQ_REL); }
-    // Only be called from the control plane work_serializer.
-    // The only place where the picker_refcount_ can be increased is
-    // RefByPicker(), which also can only be called from the control plane
-    // work_serializer. Also, if the picker_refcount_ is 0,
-    // total_requests_in_progress_ can't be increased from 0. So it's safe to
-    // delete the LocalityStats right after this method returns true.
-    bool IsSafeToDelete() {
-      return picker_refcount_.FetchAdd(0, MemoryOrder::ACQ_REL) == 0 &&
-             total_requests_in_progress_.FetchAdd(0, MemoryOrder::ACQ_REL) == 0;
+  using DroppedRequestsMap = std::map<std::string /* category */, uint64_t>;
+
+  XdsClusterDropStats(RefCountedPtr<XdsClient> xds_client,
+                      StringView lrs_server_name, StringView cluster_name,
+                      StringView eds_service_name);
+  ~XdsClusterDropStats();
+
+  // Returns a snapshot of this instance and resets all the counters.
+  DroppedRequestsMap GetSnapshotAndReset();
+
+  void AddCallDropped(const std::string& category);
+
+ private:
+  RefCountedPtr<XdsClient> xds_client_;
+  StringView lrs_server_name_;
+  StringView cluster_name_;
+  StringView eds_service_name_;
+  // Protects dropped_requests_. A mutex is necessary because the length of
+  // dropped_requests_ can be accessed by both the picker (from data plane
+  // mutex) and the load reporting thread (from the control plane combiner).
+  Mutex mu_;
+  DroppedRequestsMap dropped_requests_;
+};
+
+// Locality stats for an xds cluster.
+class XdsClusterLocalityStats : public RefCounted<XdsClusterLocalityStats> {
+ public:
+  struct BackendMetric {
+    uint64_t num_requests_finished_with_metric;
+    double total_metric_value;
+
+    BackendMetric& operator+=(const BackendMetric& other) {
+      num_requests_finished_with_metric +=
+          other.num_requests_finished_with_metric;
+      total_metric_value += other.total_metric_value;
+      return *this;
     }
 
-    void AddCallStarted();
-    void AddCallFinished(bool fail = false);
-
-   private:
-    Atomic<uint64_t> total_successful_requests_{0};
-    Atomic<uint64_t> total_requests_in_progress_{0};
-    // Requests that were issued (not dropped) but failed.
-    Atomic<uint64_t> total_error_requests_{0};
-    Atomic<uint64_t> total_issued_requests_{0};
-    // Protects load_metric_stats_. A mutex is necessary because the length of
-    // load_metric_stats_ can be accessed by both the callback intercepting the
-    // call's recv_trailing_metadata (not from any work_serializer) and the load
-    // reporting thread (from the control plane work_serializer).
-    Mutex load_metric_stats_mu_;
-    LoadMetricMap load_metric_stats_;
-    // Can be accessed from either the control plane work_serializer or the data
-    // plane mutex.
-    Atomic<uint8_t> picker_refcount_{0};
+    bool IsZero() const {
+      return num_requests_finished_with_metric == 0 && total_metric_value == 0;
+    }
   };
 
-  // TODO(juanlishen): The value type of Map<> must be movable in current
-  // implementation. To avoid making LocalityStats movable, we wrap it by
-  // std::unique_ptr<>. We should remove this wrapper if the value type of Map<>
-  // doesn't have to be movable.
-  using LocalityStatsMap =
-      std::map<RefCountedPtr<XdsLocalityName>, RefCountedPtr<LocalityStats>,
-               XdsLocalityName::Less>;
-  using LocalityStatsSnapshotMap =
-      std::map<RefCountedPtr<XdsLocalityName>, LocalityStats::Snapshot,
-               XdsLocalityName::Less>;
-  using DroppedRequestsMap = std::map<std::string, uint64_t>;
-  using DroppedRequestsSnapshotMap = DroppedRequestsMap;
-
   struct Snapshot {
-    // TODO(juanlishen): Change this to const method when const_iterator is
-    // added to Map<>.
-    bool IsAllZero();
-
-    LocalityStatsSnapshotMap upstream_locality_stats;
-    uint64_t total_dropped_requests;
-    DroppedRequestsSnapshotMap dropped_requests;
-    // The actual load report interval.
-    grpc_millis load_report_interval;
+    uint64_t total_successful_requests;
+    uint64_t total_requests_in_progress;
+    uint64_t total_error_requests;
+    uint64_t total_issued_requests;
+    std::map<std::string, BackendMetric> backend_metrics;
+
+    Snapshot& operator+=(const Snapshot& other) {
+      total_successful_requests += other.total_successful_requests;
+      total_requests_in_progress += other.total_requests_in_progress;
+      total_error_requests += other.total_error_requests;
+      total_issued_requests += other.total_issued_requests;
+      for (const auto& p : other.backend_metrics) {
+        backend_metrics[p.first] += p.second;
+      }
+      return *this;
+    }
+
+    bool IsZero() const {
+      if (total_successful_requests != 0 || total_requests_in_progress != 0 ||
+          total_error_requests != 0 || total_issued_requests != 0) {
+        return false;
+      }
+      for (const auto& p : backend_metrics) {
+        if (!p.second.IsZero()) return false;
+      }
+      return true;
+    }
   };
 
-  // Returns a snapshot of this instance and reset all the accumulative
-  // counters.
+  XdsClusterLocalityStats(RefCountedPtr<XdsClient> xds_client,
+                          StringView lrs_server_name, StringView cluster_name,
+                          StringView eds_service_name,
+                          RefCountedPtr<XdsLocalityName> name);
+  ~XdsClusterLocalityStats();
+
+  // Returns a snapshot of this instance and resets all the counters.
   Snapshot GetSnapshotAndReset();
 
-  void MaybeInitLastReportTime();
-  RefCountedPtr<LocalityStats> FindLocalityStats(
-      const RefCountedPtr<XdsLocalityName>& locality_name);
-  void PruneLocalityStats();
-  void AddCallDropped(const std::string& category);
+  void AddCallStarted();
+  void AddCallFinished(bool fail = false);
 
  private:
-  // The stats for each locality.
-  LocalityStatsMap upstream_locality_stats_;
-  Atomic<uint64_t> total_dropped_requests_{0};
-  // Protects dropped_requests_. A mutex is necessary because the length of
-  // dropped_requests_ can be accessed by both the picker (from data plane
-  // mutex) and the load reporting thread (from the control plane
-  // work_serializer).
-  Mutex dropped_requests_mu_;
-  DroppedRequestsMap dropped_requests_;
-  // The timestamp of last reporting. For the LB-policy-wide first report, the
-  // last_report_time is the time we scheduled the first reporting timer.
-  grpc_millis last_report_time_ = -1;
+  RefCountedPtr<XdsClient> xds_client_;
+  StringView lrs_server_name_;
+  StringView cluster_name_;
+  StringView eds_service_name_;
+  RefCountedPtr<XdsLocalityName> name_;
+
+  Atomic<uint64_t> total_successful_requests_{0};
+  Atomic<uint64_t> total_requests_in_progress_{0};
+  Atomic<uint64_t> total_error_requests_{0};
+  Atomic<uint64_t> total_issued_requests_{0};
+
+  // Protects backend_metrics_. A mutex is necessary because the length of
+  // backend_metrics_ can be accessed by both the callback intercepting the
+  // call's recv_trailing_metadata (not from the control plane work serializer)
+  // and the load reporting thread (from the control plane work serializer).
+  Mutex backend_metrics_mu_;
+  std::map<std::string, BackendMetric> backend_metrics_;
 };
 
 }  // namespace grpc_core

src/core/ext/filters/message_size/message_size_filter.cc

@@ -86,13 +86,13 @@ MessageSizeParser::ParsePerMethodParams(const Json& json, grpc_error** error) {
     *error = GRPC_ERROR_CREATE_FROM_VECTOR("Message size parser", &error_list);
     return nullptr;
   }
-  return grpc_core::MakeUnique<MessageSizeParsedConfig>(
-      max_request_message_bytes, max_response_message_bytes);
+  return absl::make_unique<MessageSizeParsedConfig>(max_request_message_bytes,
+                                                    max_response_message_bytes);
 }
 
 void MessageSizeParser::Register() {
   g_message_size_parser_index =
-      ServiceConfig::RegisterParser(grpc_core::MakeUnique<MessageSizeParser>());
+      ServiceConfig::RegisterParser(absl::make_unique<MessageSizeParser>());
 }
 
 size_t MessageSizeParser::ParserIndex() { return g_message_size_parser_index; }

src/core/ext/transport/chttp2/server/chttp2_server.cc

@@ -31,6 +31,8 @@
 #include <grpc/support/string_util.h>
 #include <grpc/support/sync.h>
 
+#include "absl/strings/str_format.h"
+
 #include "src/core/ext/filters/http/server/http_server_filter.h"
 #include "src/core/ext/transport/chttp2/transport/chttp2_transport.h"
 #include "src/core/ext/transport/chttp2/transport/internal.h"
@@ -413,14 +415,9 @@ grpc_error* grpc_chttp2_server_add_port(grpc_server* server, const char* addr,
 
   arg = grpc_channel_args_find(args, GRPC_ARG_ENABLE_CHANNELZ);
   if (grpc_channel_arg_get_bool(arg, GRPC_ENABLE_CHANNELZ_DEFAULT)) {
-    char* socket_name = nullptr;
-    gpr_asprintf(&socket_name, "chttp2 listener %s", addr);
     state->channelz_listen_socket =
         grpc_core::MakeRefCounted<grpc_core::channelz::ListenSocketNode>(
-            addr, socket_name);
-    // TODO(veblush): Remove this once gpr_asprintf is replaced by
-    // absl::StrFormat
-    gpr_free(socket_name);
+            addr, absl::StrFormat("chttp2 listener %s", addr));
   }
 
   /* Register with the server only upon success */

src/core/ext/transport/chttp2/transport/chttp2_transport.cc

@@ -31,6 +31,8 @@
 #include <grpc/support/log.h>
 #include <grpc/support/string_util.h>
 
+#include "absl/strings/str_format.h"
+
 #include "src/core/ext/transport/chttp2/transport/context_list.h"
 #include "src/core/ext/transport/chttp2/transport/frame_data.h"
 #include "src/core/ext/transport/chttp2/transport/internal.h"
@@ -378,14 +380,10 @@ static bool read_channel_args(grpc_chttp2_transport* t,
   if (channelz_enabled) {
     // TODO(ncteisen): add an API to endpoint to query for local addr, and pass
     // it in here, so SocketNode knows its own address.
-    char* socket_name = nullptr;
-    gpr_asprintf(&socket_name, "%s %s", get_vtable()->name, t->peer_string);
     t->channelz_socket =
         grpc_core::MakeRefCounted<grpc_core::channelz::SocketNode>(
-            "", t->peer_string, socket_name);
-    // TODO(veblush): Remove this once gpr_asprintf is replaced by
-    // absl::StrFormat
-    gpr_free(socket_name);
+            "", t->peer_string,
+            absl::StrFormat("%s %s", get_vtable()->name, t->peer_string));
   }
   return enable_bdp;
 }

src/core/lib/channel/channelz.h

@@ -148,17 +148,13 @@ class CallCountingHelper {
     // Make sure the size is exactly one cache line.
     uint8_t padding[GPR_CACHELINE_SIZE - 3 * sizeof(Atomic<intptr_t>) -
                     sizeof(Atomic<gpr_cycle_counter>)];
-  }
-#if GRPC_USE_ABSL
+  };
   // TODO(soheilhy,veblush): Revist this after abseil integration.
   // This has a problem when using abseil inlined_vector because it
   // carries an alignment attribute properly but our allocator doesn't
   // respect this. To avoid UBSAN errors, this should be removed with
   // abseil inlined_vector.
-  ;
-#else
-  GPR_ALIGN_STRUCT(GPR_CACHELINE_SIZE);
-#endif
+  // GPR_ALIGN_STRUCT(GPR_CACHELINE_SIZE);
 
   struct CounterData {
     int64_t calls_started = 0;

src/core/lib/gprpp/inlined_vector.h

@@ -24,229 +24,14 @@
 #include <cassert>
 #include <cstring>
 
-#include "src/core/lib/gprpp/memory.h"
-
-#if GRPC_USE_ABSL
 #include "absl/container/inlined_vector.h"
-#endif
+#include "src/core/lib/gprpp/memory.h"
 
 namespace grpc_core {
 
-#if GRPC_USE_ABSL
-
 template <typename T, size_t N, typename A = std::allocator<T>>
 using InlinedVector = absl::InlinedVector<T, N, A>;
 
-#else
-
-// NOTE: We eventually want to use absl::InlinedVector here.  However,
-// there are currently build problems that prevent us from using absl.
-// In the interim, we define a custom implementation as a place-holder,
-// with the intent to eventually replace this with the absl
-// implementation.
-//
-// This place-holder implementation does not implement the full set of
-// functionality from the absl version; it has just the methods that we
-// currently happen to need in gRPC.  If additional functionality is
-// needed before this gets replaced with the absl version, it can be
-// added, with the following proviso:
-//
-// ANY METHOD ADDED HERE MUST COMPLY WITH THE INTERFACE IN THE absl
-// IMPLEMENTATION!
-//
-// TODO(nnoble, roth): Replace this with absl::InlinedVector once we
-// integrate absl into the gRPC build system in a usable way.
-template <typename T, size_t N>
-class InlinedVector {
- public:
-  InlinedVector() { init_data(); }
-  ~InlinedVector() { destroy_elements(); }
-
-  // copy constructor
-  InlinedVector(const InlinedVector& v) {
-    init_data();
-    copy_from(v);
-  }
-
-  InlinedVector& operator=(const InlinedVector& v) {
-    if (this != &v) {
-      clear();
-      copy_from(v);
-    }
-    return *this;
-  }
-
-  // move constructor
-  InlinedVector(InlinedVector&& v) {
-    init_data();
-    move_from(v);
-  }
-
-  InlinedVector& operator=(InlinedVector&& v) {
-    if (this != &v) {
-      clear();
-      move_from(v);
-    }
-    return *this;
-  }
-
-  T* data() {
-    return dynamic_ != nullptr ? dynamic_ : reinterpret_cast<T*>(inline_);
-  }
-
-  const T* data() const {
-    return dynamic_ != nullptr ? dynamic_ : reinterpret_cast<const T*>(inline_);
-  }
-
-  T& operator[](size_t offset) {
-    assert(offset < size_);
-    return data()[offset];
-  }
-
-  const T& operator[](size_t offset) const {
-    assert(offset < size_);
-    return data()[offset];
-  }
-
-  bool operator==(const InlinedVector& other) const {
-    if (size_ != other.size_) return false;
-    for (size_t i = 0; i < size_; ++i) {
-      // Note that this uses == instead of != so that the data class doesn't
-      // have to implement !=.
-      if (!(data()[i] == other.data()[i])) return false;
-    }
-    return true;
-  }
-
-  bool operator!=(const InlinedVector& other) const {
-    return !(*this == other);
-  }
-
-  void reserve(size_t capacity) {
-    if (capacity > capacity_) {
-      T* new_dynamic =
-          std::alignment_of<T>::value == 0
-              ? static_cast<T*>(gpr_malloc(sizeof(T) * capacity))
-              : static_cast<T*>(gpr_malloc_aligned(
-                    sizeof(T) * capacity, std::alignment_of<T>::value));
-      move_elements(data(), new_dynamic, size_);
-      free_dynamic();
-      dynamic_ = new_dynamic;
-      capacity_ = capacity;
-    }
-  }
-
-  void resize(size_t new_size) {
-    while (new_size > size_) emplace_back();
-    while (new_size < size_) pop_back();
-  }
-
-  template <typename... Args>
-  void emplace_back(Args&&... args) {
-    if (size_ == capacity_) {
-      reserve(capacity_ * 2);
-    }
-    new (&(data()[size_])) T(std::forward<Args>(args)...);
-    ++size_;
-  }
-
-  void push_back(const T& value) { emplace_back(value); }
-
-  void push_back(T&& value) { emplace_back(std::move(value)); }
-
-  void pop_back() {
-    assert(!empty());
-    size_t s = size();
-    T& value = data()[s - 1];
-    value.~T();
-    size_--;
-  }
-
-  T& front() { return data()[0]; }
-  const T& front() const { return data()[0]; }
-
-  T& back() { return data()[size_ - 1]; }
-  const T& back() const { return data()[size_ - 1]; }
-
-  size_t size() const { return size_; }
-  bool empty() const { return size_ == 0; }
-
-  size_t capacity() const { return capacity_; }
-
-  void clear() {
-    destroy_elements();
-    init_data();
-  }
-
- private:
-  void copy_from(const InlinedVector& v) {
-    // if v is allocated, make sure we have enough capacity.
-    if (v.dynamic_ != nullptr) {
-      reserve(v.capacity_);
-    }
-    // copy over elements
-    for (size_t i = 0; i < v.size_; ++i) {
-      new (&(data()[i])) T(v[i]);
-    }
-    // copy over metadata
-    size_ = v.size_;
-    capacity_ = v.capacity_;
-  }
-
-  void move_from(InlinedVector& v) {
-    // if v is allocated, then we steal its dynamic array; otherwise, we
-    // move the elements individually.
-    if (v.dynamic_ != nullptr) {
-      dynamic_ = v.dynamic_;
-    } else {
-      move_elements(v.data(), data(), v.size_);
-    }
-    // copy over metadata
-    size_ = v.size_;
-    capacity_ = v.capacity_;
-    // null out the original
-    v.init_data();
-  }
-
-  static void move_elements(T* src, T* dst, size_t num_elements) {
-    for (size_t i = 0; i < num_elements; ++i) {
-      new (&dst[i]) T(std::move(src[i]));
-      src[i].~T();
-    }
-  }
-
-  void init_data() {
-    dynamic_ = nullptr;
-    size_ = 0;
-    capacity_ = N;
-  }
-
-  void destroy_elements() {
-    for (size_t i = 0; i < size_; ++i) {
-      T& value = data()[i];
-      value.~T();
-    }
-    free_dynamic();
-  }
-
-  void free_dynamic() {
-    if (dynamic_ != nullptr) {
-      if (std::alignment_of<T>::value == 0) {
-        gpr_free(dynamic_);
-      } else {
-        gpr_free_aligned(dynamic_);
-      }
-    }
-  }
-
-  typename std::aligned_storage<sizeof(T)>::type inline_[N];
-  T* dynamic_;
-  size_t size_;
-  size_t capacity_;
-};
-
-#endif
-
 }  // namespace grpc_core
 
 #endif /* GRPC_CORE_LIB_GPRPP_INLINED_VECTOR_H */

src/core/lib/gprpp/memory.h

@@ -28,6 +28,8 @@
 #include <memory>
 #include <utility>
 
+#include "absl/memory/memory.h"
+
 namespace grpc_core {
 
 class DefaultDeleteChar {
@@ -44,12 +46,6 @@ class DefaultDeleteChar {
 template <typename T>
 using UniquePtr = std::unique_ptr<T, DefaultDeleteChar>;
 
-// TODO(veblush): Replace this with absl::make_unique once abseil is added.
-template <typename T, typename... Args>
-inline std::unique_ptr<T> MakeUnique(Args&&... args) {
-  return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
-}
-
 }  // namespace grpc_core
 
 #endif /* GRPC_CORE_LIB_GPRPP_MEMORY_H */

src/core/lib/gprpp/optional.h

@@ -21,8 +21,6 @@
 
 #include <grpc/support/port_platform.h>
 
-#if GRPC_USE_ABSL
-
 #include "absl/types/optional.h"
 
 namespace grpc_core {
@@ -32,39 +30,4 @@ using Optional = absl::optional<T>;
 
 }  // namespace grpc_core
 
-#else
-
-#include <utility>
-
-namespace grpc_core {
-
-/* A make-shift alternative for absl::Optional. This can be removed in favor of
- * that once absl dependencies can be introduced. */
-template <typename T>
-class Optional {
- public:
-  Optional() : value_() {}
-
-  template <typename... Args>
-  T& emplace(Args&&... args) {
-    value_ = T(std::forward<Args>(args)...);
-    set_ = true;
-    return value_;
-  }
-
-  bool has_value() const { return set_; }
-
-  void reset() { set_ = false; }
-
-  T value() const { return value_; }
-
- private:
-  T value_;
-  bool set_ = false;
-};
-
-} /* namespace grpc_core */
-
-#endif
-
 #endif /* GRPC_CORE_LIB_GPRPP_OPTIONAL_H */

src/core/lib/gprpp/string_view.h

@@ -30,134 +30,20 @@
 #include <limits>
 #include <string>
 
+#include "absl/strings/string_view.h"
 #include "src/core/lib/gpr/string.h"
 #include "src/core/lib/gpr/useful.h"
 #include "src/core/lib/gprpp/memory.h"
 
-#if GRPC_USE_ABSL
-#include "absl/strings/string_view.h"
-#endif
-
 namespace grpc_core {
 
-#if GRPC_USE_ABSL
-
 using StringView = absl::string_view;
 
-#else
-
-// Provides a light-weight view over a char array or a slice, similar but not
-// identical to absl::string_view.
-//
-// Any method that has the same name as absl::string_view MUST HAVE identical
-// semantics to what absl::string_view provides.
-//
-// Methods that are not part of absl::string_view API, must be clearly
-// annotated.
-//
-// StringView does not own the buffers that back the view. Callers must ensure
-// the buffer stays around while the StringView is accessible.
-//
-// Pass StringView by value in functions, since it is exactly two pointers in
-// size.
-//
-// The interface used here is not identical to absl::string_view. Notably, we
-// need to support slices while we cannot support std::string, and gpr string
-// style functions such as strdup() and cmp(). Once we switch to
-// absl::string_view this class will inherit from absl::string_view and add the
-// gRPC-specific APIs.
-class StringView final {
- public:
-  static constexpr size_t npos = std::numeric_limits<size_t>::max();
-
-  constexpr StringView(const char* ptr, size_t size) : ptr_(ptr), size_(size) {}
-  constexpr StringView(const char* ptr)
-      : StringView(ptr, ptr == nullptr ? 0 : strlen(ptr)) {}
-  constexpr StringView() : StringView(nullptr, 0) {}
-
-  template <typename Allocator>
-  StringView(
-      const std::basic_string<char, std::char_traits<char>, Allocator>& str)
-      : StringView(str.data(), str.size()) {}
-
-  constexpr const char* data() const { return ptr_; }
-  constexpr size_t size() const { return size_; }
-  constexpr bool empty() const { return size_ == 0; }
-
-  StringView substr(size_t start, size_t size = npos) {
-    GPR_DEBUG_ASSERT(start + size <= size_);
-    return StringView(ptr_ + start, std::min(size, size_ - start));
-  }
-
-  constexpr const char& operator[](size_t i) const { return ptr_[i]; }
-
-  const char& front() const { return ptr_[0]; }
-  const char& back() const { return ptr_[size_ - 1]; }
-
-  void remove_prefix(size_t n) {
-    GPR_DEBUG_ASSERT(n <= size_);
-    ptr_ += n;
-    size_ -= n;
-  }
-
-  void remove_suffix(size_t n) {
-    GPR_DEBUG_ASSERT(n <= size_);
-    size_ -= n;
-  }
-
-  size_t find(char c, size_t pos = 0) const {
-    if (empty() || pos >= size_) return npos;
-    const char* result =
-        static_cast<const char*>(memchr(ptr_ + pos, c, size_ - pos));
-    return result != nullptr ? result - ptr_ : npos;
-  }
-
-  void clear() {
-    ptr_ = nullptr;
-    size_ = 0;
-  }
-
-  // Converts to `std::basic_string`.
-  template <typename Allocator>
-  explicit operator std::basic_string<char, std::char_traits<char>, Allocator>()
-      const {
-    if (data() == nullptr) return {};
-    return std::basic_string<char, std::char_traits<char>, Allocator>(data(),
-                                                                      size());
-  }
-
-  // Compares with other.
-  inline int compare(StringView other) {
-    const size_t len = GPR_MIN(size(), other.size());
-    const int ret = strncmp(data(), other.data(), len);
-    if (ret != 0) return ret;
-    if (size() == other.size()) return 0;
-    if (size() < other.size()) return -1;
-    return 1;
-  }
-
- private:
-  const char* ptr_;
-  size_t size_;
-};
-
-inline bool operator==(StringView lhs, StringView rhs) {
-  return lhs.size() == rhs.size() &&
-         strncmp(lhs.data(), rhs.data(), lhs.size()) == 0;
-}
-
-inline bool operator!=(StringView lhs, StringView rhs) { return !(lhs == rhs); }
-
-inline bool operator<(StringView lhs, StringView rhs) {
-  return lhs.compare(rhs) < 0;
-}
-
-#endif  // GRPC_USE_ABSL
-
 // Converts grpc_slice to StringView.
-inline StringView StringViewFromSlice(const grpc_slice& slice) {
-  return StringView(reinterpret_cast<const char*>(GRPC_SLICE_START_PTR(slice)),
-                    GRPC_SLICE_LENGTH(slice));
+inline absl::string_view StringViewFromSlice(const grpc_slice& slice) {
+  return absl::string_view(
+      reinterpret_cast<const char*>(GRPC_SLICE_START_PTR(slice)),
+      GRPC_SLICE_LENGTH(slice));
 }
 
 // Creates a dup of the string viewed by this class.

src/core/lib/iomgr/ev_epollex_linux.cc

@@ -41,11 +41,11 @@
 #include <grpc/support/alloc.h>
 #include <grpc/support/string_util.h>
 
+#include "absl/container/inlined_vector.h"
 #include "src/core/lib/debug/stats.h"
 #include "src/core/lib/gpr/spinlock.h"
 #include "src/core/lib/gpr/tls.h"
 #include "src/core/lib/gpr/useful.h"
-#include "src/core/lib/gprpp/inlined_vector.h"
 #include "src/core/lib/gprpp/manual_constructor.h"
 #include "src/core/lib/gprpp/ref_counted.h"
 #include "src/core/lib/gprpp/sync.h"
@@ -190,7 +190,15 @@ struct grpc_fd {
     grpc_iomgr_unregister_object(&iomgr_object);
 
     POLLABLE_UNREF(pollable_obj, "fd_pollable");
-    pollset_fds.clear();
+
+    // To clear out the allocations of pollset_fds, we need to swap its
+    // contents with a newly-constructed (and soon to be destructed) local
+    // variable of its same type. This is because InlinedVector::clear is _not_
+    // guaranteed to actually free up allocations and this is important since
+    // this object doesn't have a conventional destructor.
+    absl::InlinedVector<int, 1> pollset_fds_tmp;
+    pollset_fds_tmp.swap(pollset_fds);
+
     gpr_mu_destroy(&pollable_mu);
     gpr_mu_destroy(&orphan_mu);
 
@@ -232,8 +240,8 @@ struct grpc_fd {
 
   // Protects pollable_obj and pollset_fds.
   gpr_mu pollable_mu;
-  grpc_core::InlinedVector<int, 1> pollset_fds;  // Used in PO_MULTI.
-  pollable* pollable_obj = nullptr;              // Used in PO_FD.
+  absl::InlinedVector<int, 1> pollset_fds;  // Used in PO_MULTI.
+  pollable* pollable_obj = nullptr;         // Used in PO_FD.
 
   grpc_core::LockfreeEvent read_closure;
   grpc_core::LockfreeEvent write_closure;

src/core/lib/iomgr/tcp_posix.cc

@@ -1385,7 +1385,8 @@ static bool do_tcp_flush_zerocopy(grpc_tcp* tcp, TcpZerocopySendRecord* record,
 
 static void UnrefMaybePutZerocopySendRecord(grpc_tcp* tcp,
                                             TcpZerocopySendRecord* record,
-                                            uint32_t seq, const char* tag) {
+                                            uint32_t seq,
+                                            const char* /* tag */) {
   if (record->Unref()) {
     tcp->tcp_zerocopy_send_ctx.PutSendRecord(record);
   }

src/core/lib/security/credentials/credentials.h

@@ -56,6 +56,7 @@ typedef enum {
 #define GRPC_CALL_CREDENTIALS_TYPE_COMPOSITE "Composite"
 
 #define GRPC_AUTHORIZATION_METADATA_KEY "authorization"
+#define GRPC_AUTH_QUOTA_PROJECT_METADATA_KEY "x-goog-user-project"
 #define GRPC_IAM_AUTHORIZATION_TOKEN_METADATA_KEY \
   "x-goog-iam-authorization-token"
 #define GRPC_IAM_AUTHORITY_SELECTOR_METADATA_KEY "x-goog-iam-authority-selector"

src/core/lib/security/credentials/oauth2/oauth2_credentials.cc

@@ -74,6 +74,9 @@ grpc_auth_refresh_token grpc_auth_refresh_token_create_from_json(
   }
   result.type = GRPC_AUTH_JSON_TYPE_AUTHORIZED_USER;
 
+  // quota_project_id is optional, so we don't check the result of the copy.
+  grpc_copy_json_string_property(json, "quota_project_id",
+                                 &result.quota_project_id);
   if (!grpc_copy_json_string_property(json, "client_secret",
                                       &result.client_secret) ||
       !grpc_copy_json_string_property(json, "client_id", &result.client_id) ||
@@ -114,6 +117,10 @@ void grpc_auth_refresh_token_destruct(grpc_auth_refresh_token* refresh_token) {
     gpr_free(refresh_token->refresh_token);
     refresh_token->refresh_token = nullptr;
   }
+  if (refresh_token->quota_project_id != nullptr) {
+    gpr_free(refresh_token->quota_project_id);
+    refresh_token->quota_project_id = nullptr;
+  }
 }
 
 //
@@ -276,6 +283,7 @@ bool grpc_oauth2_token_fetcher_credentials::get_request_metadata(
     grpc_polling_entity* pollent, grpc_auth_metadata_context /*context*/,
     grpc_credentials_mdelem_array* md_array, grpc_closure* on_request_metadata,
     grpc_error** /*error*/) {
+  maybe_add_additional_metadata(md_array);
   // Check if we can use the cached token.
   grpc_millis refresh_threshold =
       GRPC_SECURE_TOKEN_REFRESH_THRESHOLD_SECS * GPR_MS_PER_SEC;
@@ -453,6 +461,17 @@ void grpc_google_refresh_token_credentials::fetch_oauth2(
   gpr_free(body);
 }
 
+void grpc_google_refresh_token_credentials::maybe_add_additional_metadata(
+    grpc_credentials_mdelem_array* md_array) {
+  if (refresh_token_.quota_project_id != nullptr) {
+    grpc_mdelem quota_project_md = grpc_mdelem_from_slices(
+        grpc_core::ExternallyManagedSlice(GRPC_AUTH_QUOTA_PROJECT_METADATA_KEY),
+        grpc_core::ExternallyManagedSlice(refresh_token_.quota_project_id));
+    grpc_credentials_mdelem_array_add(md_array, quota_project_md);
+    GRPC_MDELEM_UNREF(quota_project_md);
+  }
+}
+
 grpc_google_refresh_token_credentials::grpc_google_refresh_token_credentials(
     grpc_auth_refresh_token refresh_token)
     : refresh_token_(refresh_token) {}

src/core/lib/security/credentials/oauth2/oauth2_credentials.h

@@ -32,12 +32,13 @@
   "s&subject_token_type=%s"
 
 // auth_refresh_token parsing.
-typedef struct {
+struct grpc_auth_refresh_token {
   const char* type;
   char* client_id;
   char* client_secret;
   char* refresh_token;
-} grpc_auth_refresh_token;
+  char* quota_project_id = nullptr;
+};
 
 /// Returns 1 if the object is valid, 0 otherwise.
 int grpc_auth_refresh_token_is_valid(
@@ -90,6 +91,9 @@ class grpc_oauth2_token_fetcher_credentials : public grpc_call_credentials {
                             grpc_httpcli_context* httpcli_context,
                             grpc_polling_entity* pollent, grpc_iomgr_cb_func cb,
                             grpc_millis deadline) = 0;
+  // Sub class may override this for adding additional metadata other than
+  // credentials itself.
+  virtual void maybe_add_additional_metadata(grpc_credentials_mdelem_array*) {}
 
  private:
   gpr_mu mu_;
@@ -117,6 +121,8 @@ class grpc_google_refresh_token_credentials final
                     grpc_httpcli_context* httpcli_context,
                     grpc_polling_entity* pollent, grpc_iomgr_cb_func cb,
                     grpc_millis deadline) override;
+  void maybe_add_additional_metadata(
+      grpc_credentials_mdelem_array* md_array) override;
 
  private:
   grpc_auth_refresh_token refresh_token_;

src/core/lib/security/security_connector/local/local_security_connector.cc

@@ -66,8 +66,7 @@ grpc_core::RefCountedPtr<grpc_auth_context> local_auth_context_create(
   return ctx;
 }
 
-void local_check_peer(grpc_security_connector* sc, tsi_peer peer,
-                      grpc_endpoint* ep,
+void local_check_peer(tsi_peer peer, grpc_endpoint* ep,
                       grpc_core::RefCountedPtr<grpc_auth_context>* auth_context,
                       grpc_closure* on_peer_checked,
                       grpc_local_connect_type type) {
@@ -178,7 +177,7 @@ class grpc_local_channel_security_connector final
                   grpc_closure* on_peer_checked) override {
     grpc_local_credentials* creds =
         reinterpret_cast<grpc_local_credentials*>(mutable_channel_creds());
-    local_check_peer(this, peer, ep, auth_context, on_peer_checked,
+    local_check_peer(peer, ep, auth_context, on_peer_checked,
                      creds->connect_type());
   }
 
@@ -227,7 +226,7 @@ class grpc_local_server_security_connector final
                   grpc_closure* on_peer_checked) override {
     grpc_local_server_credentials* creds =
         static_cast<grpc_local_server_credentials*>(mutable_server_creds());
-    local_check_peer(this, peer, ep, auth_context, on_peer_checked,
+    local_check_peer(peer, ep, auth_context, on_peer_checked,
                      creds->connect_type());
   }
 

src/core/lib/security/security_connector/ssl/ssl_security_connector.cc

@@ -190,21 +190,9 @@ class grpc_ssl_channel_security_connector final
                        grpc_auth_context* auth_context,
                        grpc_closure* /*on_call_host_checked*/,
                        grpc_error** error) override {
-    grpc_security_status status = GRPC_SECURITY_ERROR;
-    tsi_peer peer = grpc_shallow_peer_from_ssl_auth_context(auth_context);
-    if (grpc_ssl_host_matches_name(&peer, host)) status = GRPC_SECURITY_OK;
-    /* If the target name was overridden, then the original target_name was
-       'checked' transitively during the previous peer check at the end of the
-       handshake. */
-    if (overridden_target_name_ != nullptr && host == target_name_.get()) {
-      status = GRPC_SECURITY_OK;
-    }
-    if (status != GRPC_SECURITY_OK) {
-      *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING(
-          "call host does not match SSL server name");
-    }
-    grpc_shallow_peer_destruct(&peer);
-    return true;
+    return grpc_ssl_check_call_host(host, target_name_.get(),
+                                    overridden_target_name_.get(), auth_context,
+                                    error);
   }
 
   void cancel_check_call_host(grpc_closure* /*on_call_host_checked*/,

src/core/lib/security/security_connector/ssl_utils.cc

@@ -167,11 +167,13 @@ bool grpc_ssl_check_call_host(grpc_core::StringView host,
                               grpc_core::StringView target_name,
                               grpc_core::StringView overridden_target_name,
                               grpc_auth_context* auth_context,
-                              grpc_closure* /*on_call_host_checked*/,
                               grpc_error** error) {
   grpc_security_status status = GRPC_SECURITY_ERROR;
   tsi_peer peer = grpc_shallow_peer_from_ssl_auth_context(auth_context);
   if (grpc_ssl_host_matches_name(&peer, host)) status = GRPC_SECURITY_OK;
+  /* If the target name was overridden, then the original target_name was
+   'checked' transitively during the previous peer check at the end of the
+   handshake. */
   if (!overridden_target_name.empty() && host == target_name) {
     status = GRPC_SECURITY_OK;
   }

src/core/lib/security/security_connector/ssl_utils.h

@@ -57,7 +57,6 @@ bool grpc_ssl_check_call_host(grpc_core::StringView host,
                               grpc_core::StringView target_name,
                               grpc_core::StringView overridden_target_name,
                               grpc_auth_context* auth_context,
-                              grpc_closure* on_call_host_checked,
                               grpc_error** error);
 /* Return HTTP2-compliant cipher suites that gRPC accepts by default. */
 const char* grpc_get_ssl_cipher_suites(void);

src/core/lib/security/security_connector/tls/tls_security_connector.cc

@@ -268,7 +268,7 @@ bool TlsChannelSecurityConnector::check_call_host(
     grpc_closure* on_call_host_checked, grpc_error** error) {
   return grpc_ssl_check_call_host(host, target_name_.get(),
                                   overridden_target_name_.get(), auth_context,
-                                  on_call_host_checked, error);
+                                  error);
 }
 
 void TlsChannelSecurityConnector::cancel_check_call_host(

src/core/lib/security/transport/client_auth_filter.cc

@@ -165,9 +165,17 @@ static void on_credentials_metadata(void* arg, grpc_error* input_error) {
     grpc_metadata_batch* mdb =
         batch->payload->send_initial_metadata.send_initial_metadata;
     for (size_t i = 0; i < calld->md_array.size; ++i) {
-      add_error(&error, grpc_metadata_batch_add_tail(
-                            mdb, &calld->md_links[i],
-                            GRPC_MDELEM_REF(calld->md_array.md[i])));
+      // Only add x-goog-user-project header if not present.
+      if (grpc_slice_str_cmp(GRPC_MDKEY(calld->md_array.md[i]),
+                             GRPC_AUTH_QUOTA_PROJECT_METADATA_KEY) == 0) {
+        add_error(&error, grpc_metadata_batch_add_tail_when_key_not_exist(
+                              mdb, &calld->md_links[i],
+                              GRPC_MDELEM_REF(calld->md_array.md[i])));
+      } else {
+        add_error(&error, grpc_metadata_batch_add_tail(
+                              mdb, &calld->md_links[i],
+                              GRPC_MDELEM_REF(calld->md_array.md[i])));
+      }
     }
   }
   if (error == GRPC_ERROR_NONE) {

src/core/lib/security/transport/security_handshaker.cc

@@ -559,10 +559,10 @@ RefCountedPtr<Handshaker> SecurityHandshakerCreate(
 void SecurityRegisterHandshakerFactories() {
   HandshakerRegistry::RegisterHandshakerFactory(
       false /* at_start */, HANDSHAKER_CLIENT,
-      grpc_core::MakeUnique<ClientSecurityHandshakerFactory>());
+      absl::make_unique<ClientSecurityHandshakerFactory>());
   HandshakerRegistry::RegisterHandshakerFactory(
       false /* at_start */, HANDSHAKER_SERVER,
-      grpc_core::MakeUnique<ServerSecurityHandshakerFactory>());
+      absl::make_unique<ServerSecurityHandshakerFactory>());
 }
 
 }  // namespace grpc_core

src/core/lib/surface/version.cc

@@ -25,4 +25,4 @@
 
 const char* grpc_version_string(void) { return "9.0.0"; }
 
-const char* grpc_g_stands_for(void) { return "guantao"; }
+const char* grpc_g_stands_for(void) { return "galactic"; }

src/core/lib/transport/metadata_batch.cc

@@ -205,6 +205,19 @@ grpc_error* grpc_metadata_batch_add_tail(grpc_metadata_batch* batch,
   return grpc_metadata_batch_link_tail(batch, storage);
 }
 
+grpc_error* grpc_metadata_batch_add_tail_when_key_not_exist(
+    grpc_metadata_batch* batch, grpc_linked_mdelem* storage,
+    grpc_mdelem elem_to_add) {
+  auto cur = batch->list.head;
+  while (cur != nullptr) {
+    if (grpc_slice_cmp(GRPC_MDKEY(cur->md), GRPC_MDKEY(elem_to_add)) == 0) {
+      // We already have the same key, just returning.
+      return GRPC_ERROR_NONE;
+    }
+  }
+  return grpc_metadata_batch_add_tail(batch, storage, elem_to_add);
+}
+
 static void link_tail(grpc_mdelem_list* list, grpc_linked_mdelem* storage) {
   assert_valid_list(list);
   GPR_DEBUG_ASSERT(!GRPC_MDISNULL(storage->md));

src/core/lib/transport/metadata_batch.h

@@ -138,6 +138,17 @@ grpc_error* grpc_metadata_batch_add_tail(
     grpc_metadata_batch* batch, grpc_linked_mdelem* storage,
     grpc_mdelem elem_to_add) GRPC_MUST_USE_RESULT;
 
+/** Add \a elem_to_add as the last element in \a batch, only
+    when the current batch doesn't have the same key in the
+    given element, using \a storage as backing storage for the
+    linked list element.  \a storage is owned by the caller
+    and must survive for the lifetime of batch. This usually
+    means it should be around for the lifetime of the call.
+    Takes ownership of \a elem_to_add */
+grpc_error* grpc_metadata_batch_add_tail_when_key_not_exist(
+    grpc_metadata_batch* batch, grpc_linked_mdelem* storage,
+    grpc_mdelem elem_to_add) GRPC_MUST_USE_RESULT;
+
 inline grpc_error* GRPC_MUST_USE_RESULT grpc_metadata_batch_add_tail(
     grpc_metadata_batch* batch, grpc_linked_mdelem* storage,
     grpc_metadata_batch_callouts_index idx) {

src/core/tsi/alts/handshaker/alts_handshaker_client.cc

@@ -637,7 +637,7 @@ static void handshaker_client_shutdown(alts_handshaker_client* c) {
   }
 }
 
-static void handshaker_call_unref(void* arg, grpc_error* error) {
+static void handshaker_call_unref(void* arg, grpc_error* /* error */) {
   grpc_call* call = static_cast<grpc_call*>(arg);
   grpc_call_unref(call);
 }

src/core/tsi/alts/handshaker/alts_tsi_handshaker.cc

@@ -430,7 +430,7 @@ struct alts_tsi_handshaker_continue_handshaker_next_args {
 };
 
 static void alts_tsi_handshaker_create_channel(void* arg,
-                                               grpc_error* unused_error) {
+                                               grpc_error* /* unused_error */) {
   alts_tsi_handshaker_continue_handshaker_next_args* next_args =
       static_cast<alts_tsi_handshaker_continue_handshaker_next_args*>(arg);
   alts_tsi_handshaker* handshaker = next_args->handshaker;

src/core/tsi/ssl/session_cache/ssl_session_boringssl.cc

@@ -49,7 +49,7 @@ class BoringSslCachedSession : public SslCachedSession {
 
 std::unique_ptr<SslCachedSession> SslCachedSession::Create(
     SslSessionPtr session) {
-  return grpc_core::MakeUnique<BoringSslCachedSession>(std::move(session));
+  return absl::make_unique<BoringSslCachedSession>(std::move(session));
 }
 
 }  // namespace tsi

src/core/tsi/ssl/session_cache/ssl_session_openssl.cc

@@ -67,7 +67,7 @@ class OpenSslCachedSession : public SslCachedSession {
 
 std::unique_ptr<SslCachedSession> SslCachedSession::Create(
     SslSessionPtr session) {
-  return grpc_core::MakeUnique<OpenSslCachedSession>(std::move(session));
+  return absl::make_unique<OpenSslCachedSession>(std::move(session));
 }
 
 }  // namespace tsi

src/cpp/common/alts_util.cc

@@ -53,7 +53,7 @@ std::unique_ptr<AltsContext> GetAltsContextFromAuthContext(
     gpr_log(GPR_ERROR, "security_level is invalid.");
     return nullptr;
   }
-  return grpc_core::MakeUnique<AltsContext>(AltsContext(ctx));
+  return absl::make_unique<AltsContext>(AltsContext(ctx));
 }
 
 grpc::Status AltsClientAuthzCheck(

src/cpp/common/tls_credentials_options.cc

@@ -16,19 +16,18 @@
  *
  */
 
+#include <grpc/support/alloc.h>
 #include <grpcpp/security/tls_credentials_options.h>
 #include "src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h"
-
-#include <grpc/support/alloc.h>
-
 #include "src/cpp/common/tls_credentials_options_util.h"
 
 namespace grpc_impl {
 namespace experimental {
 
 /** TLS key materials config API implementation **/
-void TlsKeyMaterialsConfig::set_pem_root_certs(grpc::string pem_root_certs) {
-  pem_root_certs_ = std::move(pem_root_certs);
+void TlsKeyMaterialsConfig::set_pem_root_certs(
+    const grpc::string& pem_root_certs) {
+  pem_root_certs_ = pem_root_certs;
 }
 
 void TlsKeyMaterialsConfig::add_pem_key_cert_pair(
@@ -37,10 +36,10 @@ void TlsKeyMaterialsConfig::add_pem_key_cert_pair(
 }
 
 void TlsKeyMaterialsConfig::set_key_materials(
-    grpc::string pem_root_certs,
-    std::vector<PemKeyCertPair> pem_key_cert_pair_list) {
-  pem_key_cert_pair_list_ = std::move(pem_key_cert_pair_list);
-  pem_root_certs_ = std::move(pem_root_certs);
+    const grpc::string& pem_root_certs,
+    const std::vector<PemKeyCertPair>& pem_key_cert_pair_list) {
+  pem_key_cert_pair_list_ = pem_key_cert_pair_list;
+  pem_root_certs_ = pem_root_certs;
 }
 
 /** TLS credential reload arg API implementation **/
@@ -59,7 +58,6 @@ TlsCredentialReloadArg::~TlsCredentialReloadArg() {}
 void* TlsCredentialReloadArg::cb_user_data() const {
   return c_arg_->cb_user_data;
 }
-
 bool TlsCredentialReloadArg::is_pem_key_cert_pair_list_empty() const {
   return c_arg_->key_materials_config->pem_key_cert_pair_list().empty();
 }
@@ -85,17 +83,46 @@ void TlsCredentialReloadArg::set_pem_root_certs(
   c_arg_->key_materials_config->set_pem_root_certs(std::move(c_pem_root_certs));
 }
 
-void TlsCredentialReloadArg::add_pem_key_cert_pair(
-    TlsKeyMaterialsConfig::PemKeyCertPair pem_key_cert_pair) {
+namespace {
+
+::grpc_core::PemKeyCertPair ConvertToCorePemKeyCertPair(
+    const TlsKeyMaterialsConfig::PemKeyCertPair& pem_key_cert_pair) {
   grpc_ssl_pem_key_cert_pair* ssl_pair =
       (grpc_ssl_pem_key_cert_pair*)gpr_malloc(
           sizeof(grpc_ssl_pem_key_cert_pair));
   ssl_pair->private_key = gpr_strdup(pem_key_cert_pair.private_key.c_str());
   ssl_pair->cert_chain = gpr_strdup(pem_key_cert_pair.cert_chain.c_str());
-  ::grpc_core::PemKeyCertPair c_pem_key_cert_pair =
-      ::grpc_core::PemKeyCertPair(ssl_pair);
+  return ::grpc_core::PemKeyCertPair(ssl_pair);
+}
+
+}  //  namespace
+
+void TlsCredentialReloadArg::add_pem_key_cert_pair(
+    const TlsKeyMaterialsConfig::PemKeyCertPair& pem_key_cert_pair) {
   c_arg_->key_materials_config->add_pem_key_cert_pair(
-      std::move(c_pem_key_cert_pair));
+      ConvertToCorePemKeyCertPair(pem_key_cert_pair));
+}
+
+void TlsCredentialReloadArg::set_key_materials(
+    const grpc::string& pem_root_certs,
+    std::vector<TlsKeyMaterialsConfig::PemKeyCertPair> pem_key_cert_pair_list) {
+  /** Initialize the |key_materials_config| field of |c_arg_|, if it has not
+   *  already been done. **/
+  if (c_arg_->key_materials_config == nullptr) {
+    c_arg_->key_materials_config = grpc_tls_key_materials_config_create();
+  }
+  /** Convert |pem_key_cert_pair_list| to an inlined vector of ssl pairs. **/
+  ::grpc_core::InlinedVector<::grpc_core::PemKeyCertPair, 1>
+      c_pem_key_cert_pair_list;
+  for (const auto& key_cert_pair : pem_key_cert_pair_list) {
+    c_pem_key_cert_pair_list.emplace_back(
+        ConvertToCorePemKeyCertPair(key_cert_pair));
+  }
+  /** Populate the key materials config field of |c_arg_|. **/
+  ::grpc_core::UniquePtr<char> c_pem_root_certs(
+      gpr_strdup(pem_root_certs.c_str()));
+  c_arg_->key_materials_config->set_key_materials(std::move(c_pem_root_certs),
+                                                  c_pem_key_cert_pair_list);
 }
 
 void TlsCredentialReloadArg::set_key_materials_config(
@@ -288,6 +315,11 @@ TlsCredentialsOptions::TlsCredentialsOptions(
       c_credentials_options_, server_verification_option);
 }
 
+/** Whenever a TlsCredentialsOptions instance is created, the caller takes
+ *  ownership of the c_credentials_options_ pointer (see e.g. the implementation
+ *  of the TlsCredentials API in secure_credentials.cc). For this reason, the
+ *  TlsCredentialsOptions destructor is not responsible for freeing
+ *  c_credentials_options_. **/
 TlsCredentialsOptions::~TlsCredentialsOptions() {}
 
 }  // namespace experimental

src/cpp/common/version_cc.cc

@@ -22,5 +22,5 @@
 #include <grpcpp/grpcpp.h>
 
 namespace grpc {
-grpc::string Version() { return "1.27.0-dev"; }
+grpc::string Version() { return "1.28.0-dev"; }
 }  // namespace grpc

src/cpp/server/server_callback.cc

@@ -24,27 +24,59 @@
 namespace grpc_impl {
 namespace internal {
 
+void ServerCallbackCall::ScheduleOnDone(bool inline_ondone) {
+  if (inline_ondone) {
+    CallOnDone();
+  } else {
+    // Unlike other uses of closure, do not Ref or Unref here since at this
+    // point, all the Ref'fing and Unref'fing is done for this call.
+    grpc_core::ExecCtx exec_ctx;
+    struct ClosureWithArg {
+      grpc_closure closure;
+      ServerCallbackCall* call;
+      explicit ClosureWithArg(ServerCallbackCall* call_arg) : call(call_arg) {
+        GRPC_CLOSURE_INIT(&closure,
+                          [](void* void_arg, grpc_error*) {
+                            ClosureWithArg* arg =
+                                static_cast<ClosureWithArg*>(void_arg);
+                            arg->call->CallOnDone();
+                            delete arg;
+                          },
+                          this, grpc_schedule_on_exec_ctx);
+      }
+    };
+    ClosureWithArg* arg = new ClosureWithArg(this);
+    grpc_core::Executor::Run(&arg->closure, GRPC_ERROR_NONE);
+  }
+}
+
 void ServerCallbackCall::CallOnCancel(ServerReactor* reactor) {
   if (reactor->InternalInlineable()) {
     reactor->OnCancel();
   } else {
+    // Ref to make sure that the closure executes before the whole call gets
+    // destructed, and Unref within the closure.
     Ref();
     grpc_core::ExecCtx exec_ctx;
-    struct ClosureArg {
+    struct ClosureWithArg {
+      grpc_closure closure;
       ServerCallbackCall* call;
       ServerReactor* reactor;
+      ClosureWithArg(ServerCallbackCall* call_arg, ServerReactor* reactor_arg)
+          : call(call_arg), reactor(reactor_arg) {
+        GRPC_CLOSURE_INIT(&closure,
+                          [](void* void_arg, grpc_error*) {
+                            ClosureWithArg* arg =
+                                static_cast<ClosureWithArg*>(void_arg);
+                            arg->reactor->OnCancel();
+                            arg->call->MaybeDone();
+                            delete arg;
+                          },
+                          this, grpc_schedule_on_exec_ctx);
+      }
     };
-    ClosureArg* arg = new ClosureArg{this, reactor};
-    grpc_core::Executor::Run(GRPC_CLOSURE_CREATE(
-                                 [](void* void_arg, grpc_error*) {
-                                   ClosureArg* arg =
-                                       static_cast<ClosureArg*>(void_arg);
-                                   arg->reactor->OnCancel();
-                                   arg->call->MaybeDone();
-                                   delete arg;
-                                 },
-                                 arg, nullptr),
-                             GRPC_ERROR_NONE);
+    ClosureWithArg* arg = new ClosureWithArg(this, reactor);
+    grpc_core::Executor::Run(&arg->closure, GRPC_ERROR_NONE);
   }
 }
 

src/csharp/Grpc.Core.Api/VersionInfo.cs

@@ -33,11 +33,11 @@ namespace Grpc.Core
         /// <summary>
         /// Current <c>AssemblyFileVersion</c> of gRPC C# assemblies
         /// </summary>
-        public const string CurrentAssemblyFileVersion = "2.27.0.0";
+        public const string CurrentAssemblyFileVersion = "2.28.0.0";
 
         /// <summary>
         /// Current version of gRPC C#
         /// </summary>
-        public const string CurrentVersion = "2.27.0-dev";
+        public const string CurrentVersion = "2.28.0-dev";
     }
 }

src/csharp/build/dependencies.props

@@ -1,7 +1,7 @@
 <!-- This file is generated -->
 <Project>
   <PropertyGroup>
-    <GrpcCsharpVersion>2.27.0-dev</GrpcCsharpVersion>
+    <GrpcCsharpVersion>2.28.0-dev</GrpcCsharpVersion>
     <GoogleProtobufVersion>3.11.2</GoogleProtobufVersion>
   </PropertyGroup>
 </Project>

src/csharp/build_unitypackage.bat

@@ -13,7 +13,7 @@
 @rem limitations under the License.
 
 @rem Current package versions
-set VERSION=2.27.0-dev
+set VERSION=2.28.0-dev
 
 @rem Adjust the location of nuget.exe
 set NUGET=C:\nuget\nuget.exe

src/objective-c/!ProtoCompiler-gRPCCppPlugin.podspec

@@ -42,7 +42,7 @@ Pod::Spec.new do |s|
   # exclamation mark ensures that other "regular" pods will be able to find it as it'll be installed
   # before them.
   s.name     = '!ProtoCompiler-gRPCCppPlugin'
-  v = '1.27.0-dev'
+  v = '1.28.0-dev'
   s.version  = v
   s.summary  = 'The gRPC ProtoC plugin generates C++ files from .proto services.'
   s.description = <<-DESC

src/objective-c/!ProtoCompiler-gRPCPlugin.podspec

@@ -42,7 +42,7 @@ Pod::Spec.new do |s|
   # exclamation mark ensures that other "regular" pods will be able to find it as it'll be installed
   # before them.
   s.name     = '!ProtoCompiler-gRPCPlugin'
-  v = '1.27.0-dev'
+  v = '1.28.0-dev'
   s.version  = v
   s.summary  = 'The gRPC ProtoC plugin generates Objective-C files from .proto services.'
   s.description = <<-DESC

src/objective-c/GRPCClient/version.h

@@ -22,4 +22,4 @@
 // instead. This file can be regenerated from the template by running
 // `tools/buildgen/generate_projects.sh`.
 
-#define GRPC_OBJC_VERSION_STRING @"1.27.0-dev"
+#define GRPC_OBJC_VERSION_STRING @"1.28.0-dev"

src/objective-c/tests/version.h

@@ -22,5 +22,5 @@
 // instead. This file can be regenerated from the template by running
 // `tools/buildgen/generate_projects.sh`.
 
-#define GRPC_OBJC_VERSION_STRING @"1.27.0-dev"
+#define GRPC_OBJC_VERSION_STRING @"1.28.0-dev"
 #define GRPC_C_VERSION_STRING @"9.0.0"

src/php/composer.json

@@ -2,7 +2,7 @@
   "name": "grpc/grpc-dev",
   "description": "gRPC library for PHP - for Development use only",
   "license": "Apache-2.0",
-  "version": "1.27.0",
+  "version": "1.28.0",
   "require": {
     "php": ">=5.5.0",
     "google/protobuf": "^v3.3.0"

src/php/ext/grpc/version.h

@@ -20,6 +20,6 @@
 #ifndef VERSION_H
 #define VERSION_H
 
-#define PHP_GRPC_VERSION "1.27.0dev"
+#define PHP_GRPC_VERSION "1.28.0dev"
 
 #endif /* VERSION_H */