From 144b59f91ee7eca916e6167b5c82f50400c75906 Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Fri, 27 Aug 2021 11:54:17 +0100 Subject: [PATCH 1/3] Skip the final GC on shutdown to improve restart times Signed-off-by: Sean Quah --- changelog.d/10712.feature | 1 + synapse/app/_base.py | 2 + synapse/app/_skip_final_gc.py | 83 +++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+) create mode 100644 changelog.d/10712.feature create mode 100644 synapse/app/_skip_final_gc.py diff --git a/changelog.d/10712.feature b/changelog.d/10712.feature new file mode 100644 index 000000000000..d04db6f26feb --- /dev/null +++ b/changelog.d/10712.feature @@ -0,0 +1 @@ +Skip final GC at shutdown to improve restart performance. diff --git a/synapse/app/_base.py b/synapse/app/_base.py index 39e28aff9fb6..e269c66c846b 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -34,6 +34,7 @@ import synapse from synapse.api.constants import MAX_PDU_SIZE from synapse.app import check_bind_error +from synapse.app._skip_final_gc import disable_final_gc from synapse.app.phone_stats_home import start_phone_stats_home from synapse.config.homeserver import HomeServerConfig from synapse.crypto import context_factory @@ -126,6 +127,7 @@ def run(): change_resource_limit(soft_file_limit) if gc_thresholds: gc.set_threshold(*gc_thresholds) + disable_final_gc() run_command() # make sure that we run the reactor with the sentinel log context, diff --git a/synapse/app/_skip_final_gc.py b/synapse/app/_skip_final_gc.py new file mode 100644 index 000000000000..d16cee02a572 --- /dev/null +++ b/synapse/app/_skip_final_gc.py @@ -0,0 +1,83 @@ +# Copyright 2017 New Vector Ltd +# Copyright 2019-2021 The Matrix.org Foundation C.I.C +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import atexit +import logging +import os +import sys +from typing import NoReturn + +logger = logging.getLogger(__name__) + +skip_final_gc_enabled = False + + +def _maybe_skip_final_gc() -> None: + """An `atexit` handler that skips any final garbage collection + + By calling `os._exit()` directly, Python's final garbage collection is skipped, + along with any further `atexit` handlers. This has the potentially undesirable side + effect of overriding the original exit code, so the skip should be disabled when we + think the process is terminating abnormally. + """ + if skip_final_gc_enabled: + logger.info("Skipping final GC and further exit handlers...") + + # The `logging` module's `atexit` handler is going to be skipped unless we call + # it manually here. It's responsible for flushing buffers so that any final log + # messages are not lost. + logging.shutdown() + + os._exit(0) + + +# Register our `atexit` handler early-ish, to minimize the number of handlers skipped. +# The `logging.shutdown` handler will have been registered before us unfortunately. +atexit.register(_maybe_skip_final_gc) + + +_original_sys_excepthook = sys.excepthook +_original_sys_exit = sys.exit + + +def _sys_exit(arg=None) -> NoReturn: + """Disable the skip of the final GC if exiting with a non-zero exit code + + Ensures that non-zero exit codes are preserved.""" + global skip_final_gc_enabled + if arg is None or (type(arg) == int and arg == 0): + # Exit code is 0 + pass + else: + skip_final_gc_enabled = False + _original_sys_exit(arg) + + +def _sys_excepthook(*args) -> None: + """Disable the skip of the final GC when an unhandled exception occurs + + Ensures that the non-zero exit code is preserved.""" + global skip_final_gc_enabled + skip_final_gc_enabled = False + _original_sys_excepthook(*args) + + +sys.excepthook = _sys_excepthook +sys.exit = _sys_exit + + +def disable_final_gc() -> None: + """Enable the skip of the final GC when Python exits""" + global skip_final_gc_enabled + skip_final_gc_enabled = True From 692479df9263424249d358e39769672fabb16142 Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Fri, 27 Aug 2021 13:33:44 +0100 Subject: [PATCH 2/3] Use `gc.freeze()` to speed up the final GC instead Signed-off-by: Sean Quah --- synapse/app/_base.py | 10 +++-- synapse/app/_skip_final_gc.py | 83 ----------------------------------- 2 files changed, 7 insertions(+), 86 deletions(-) delete mode 100644 synapse/app/_skip_final_gc.py diff --git a/synapse/app/_base.py b/synapse/app/_base.py index e269c66c846b..b644c5bc0340 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -12,6 +12,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import atexit import gc import logging import os @@ -34,7 +35,6 @@ import synapse from synapse.api.constants import MAX_PDU_SIZE from synapse.app import check_bind_error -from synapse.app._skip_final_gc import disable_final_gc from synapse.app.phone_stats_home import start_phone_stats_home from synapse.config.homeserver import HomeServerConfig from synapse.crypto import context_factory @@ -94,7 +94,6 @@ def start_worker_reactor(appname, config, run_command=reactor.run): run_command=run_command, ) - def start_reactor( appname, soft_file_limit, @@ -127,7 +126,6 @@ def run(): change_resource_limit(soft_file_limit) if gc_thresholds: gc.set_threshold(*gc_thresholds) - disable_final_gc() run_command() # make sure that we run the reactor with the sentinel log context, @@ -405,6 +403,12 @@ def run_sighup(*args, **kwargs): gc.collect() gc.freeze() + # Speed up shutdowns by freezing all allocated objects. This moves everything + # into the permanent generation and excludes them from the final GC. + # Unfortunately only works on Python 3.7 + if platform.python_implementation() == "CPython" and sys.version_info >= (3, 7): + atexit.register(gc.freeze) + def setup_sentry(hs): """Enable sentry integration, if enabled in configuration diff --git a/synapse/app/_skip_final_gc.py b/synapse/app/_skip_final_gc.py deleted file mode 100644 index d16cee02a572..000000000000 --- a/synapse/app/_skip_final_gc.py +++ /dev/null @@ -1,83 +0,0 @@ -# Copyright 2017 New Vector Ltd -# Copyright 2019-2021 The Matrix.org Foundation C.I.C -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -import atexit -import logging -import os -import sys -from typing import NoReturn - -logger = logging.getLogger(__name__) - -skip_final_gc_enabled = False - - -def _maybe_skip_final_gc() -> None: - """An `atexit` handler that skips any final garbage collection - - By calling `os._exit()` directly, Python's final garbage collection is skipped, - along with any further `atexit` handlers. This has the potentially undesirable side - effect of overriding the original exit code, so the skip should be disabled when we - think the process is terminating abnormally. - """ - if skip_final_gc_enabled: - logger.info("Skipping final GC and further exit handlers...") - - # The `logging` module's `atexit` handler is going to be skipped unless we call - # it manually here. It's responsible for flushing buffers so that any final log - # messages are not lost. - logging.shutdown() - - os._exit(0) - - -# Register our `atexit` handler early-ish, to minimize the number of handlers skipped. -# The `logging.shutdown` handler will have been registered before us unfortunately. -atexit.register(_maybe_skip_final_gc) - - -_original_sys_excepthook = sys.excepthook -_original_sys_exit = sys.exit - - -def _sys_exit(arg=None) -> NoReturn: - """Disable the skip of the final GC if exiting with a non-zero exit code - - Ensures that non-zero exit codes are preserved.""" - global skip_final_gc_enabled - if arg is None or (type(arg) == int and arg == 0): - # Exit code is 0 - pass - else: - skip_final_gc_enabled = False - _original_sys_exit(arg) - - -def _sys_excepthook(*args) -> None: - """Disable the skip of the final GC when an unhandled exception occurs - - Ensures that the non-zero exit code is preserved.""" - global skip_final_gc_enabled - skip_final_gc_enabled = False - _original_sys_excepthook(*args) - - -sys.excepthook = _sys_excepthook -sys.exit = _sys_exit - - -def disable_final_gc() -> None: - """Enable the skip of the final GC when Python exits""" - global skip_final_gc_enabled - skip_final_gc_enabled = True From 042420c600a3fbbc3af4988fadb3e2facd556990 Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Fri, 27 Aug 2021 13:35:18 +0100 Subject: [PATCH 3/3] lint --- synapse/app/_base.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/app/_base.py b/synapse/app/_base.py index b644c5bc0340..6fc14930d1cd 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -94,6 +94,7 @@ def start_worker_reactor(appname, config, run_command=reactor.run): run_command=run_command, ) + def start_reactor( appname, soft_file_limit,