Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support custom asynchronous logger and reuse thread pool #3293

Closed
wants to merge 4 commits into from

Conversation

F1F88
Copy link
Contributor

@F1F88 F1F88 commented Dec 12, 2024

In the current version of spdlog, customizing async_logger and reusing spdlog::thread_pool becomes difficult due to the following limitations:

  1. The post_log and post_flush parameters of the spdlog::thread_pool class are limited to the spdlog::async_logger type, which lacks flexibility.

  2. spdlog::async_logger is a final class and cannot be inherited or customized. This further limits the extensibility and reuse capabilities of the logger.

Changes

  • Abstract backend worker interface

    1. Abstract the backend_sink_it_ and backend_flush__ interfaces into a new class backend_worker.

    2. The spdlog::async_logger class inherits and implements backend_worker, and is no longer directly bound to the backend worker interface.

  • Improve the versatility of spdlog::thread_pool

    1. Modify the parameter types of post_log and post_flush from spdlog::async_logger to the general backend_worker type.

    2. Allow collaboration with other backend_worker implementations to improve the reusability of thread_pool.

Advantages

  1. Support custom asynchronous logger

    Users can easily create custom asynchronous logger by inheriting the spdlog::logger class and the backend_worker class, without being restricted by the fixed implementation of spdlog::async_logger.

  2. Enhance the reusability of spdlog::thread_pool

    spdlog::thread_pool is no longer limited to collaboration with spdlog::async_logger, but can cooperate with more implementation classes of the backend_worker class, significantly enhancing versatility and extensibility.

  3. This shouldn't be too disruptive to the original code structure, spdlog::async_logger is still the final class and can still be passed as a backend worker to spdlog::thread_pool .

@F1F88
Copy link
Contributor Author

F1F88 commented Dec 12, 2024

Off-topic

I really think spdlog is a very good project, and I've wrapped some of spdlog's APIs into SourceMod's extensions to ease the burden of logging and debugging for SourceMod plugin developers. The wrapped spdlog extension is much better than SourceMod's native logging API in terms of functionality and efficiency. Especially in terms of performance, the sdplog extension is many times faster than the SourceMod native logging API in my benchmarks.

But SourceMod is a somewhat old project, and there are a lot of problems that need to be solved by extending and wrapping spdlog. So I'm submitting this pull request to enhance spdlog's extensibility and reusability to provide more flexibility for developers.

@F1F88
Copy link
Contributor Author

F1F88 commented Dec 13, 2024

Suppose we add an option SPDLOG_CUSTOM_ASYNC_LOGGER to tweakme.h.
For developers who do not need a custom asynchronous logger, comment SPDLOG_CUSTOM_ASYNC_LOGGER (default) to keep the original code architecture (spdlog::async_logger is bound to backend_worker) to avoid the performance impact of using virtual functions.
For developers who need a custom asynchronous logger, uncomment SPDLOG_CUSTOM_ASYNC_LOGGER to abstract the backend_worker interface for greater extensibility.
Is this solution more suitable?

@gabime
Copy link
Owner

gabime commented Dec 13, 2024

Thanks @F1F88 for the suggestions. I tend not to change anything significant in this branch though.
For new development we have the v2.x branch. As for the suggestions, the approach I am taking now is to encapsulate all async in a special sink rather than a logger. This way we can have just one kind logger (and can remove a its virtual function). See the wip in this branch, specifically the new async sink.

@F1F88
Copy link
Contributor Author

F1F88 commented Dec 13, 2024

The design ideas of async-sink are impressive! I am glad to see that the async-sink branch is exploring new asynchronous function implementations.

I understand the emphasis on stability in the current branch and the decision not to make major changes in this branch. For the current needs of my project, I will temporarily modify the source code of spdlog to meet specific use cases. Although this approach may have limitations in long-term maintainability, I will closely follow the progress of async-sink and v2.x branches and re-evaluate and optimize my implementation when appropriate.

Thank you for your valuable feedback and guidance. I look forward to the official release of v2.x and hope to have the opportunity to contribute to its development in the future!

@gabime
Copy link
Owner

gabime commented Dec 13, 2024

Thank you for your valuable feedback and guidance. I look forward to the official release of v2.x and hope to have the opportunity to contribute to its development in the future!

You're welcome . Thanks @F1F88

@gabime gabime closed this Dec 13, 2024
F1F88 added a commit to F1F88/sm-ext-log4sp that referenced this pull request Dec 14, 2024
* 抽象后台工作接口,而不是将 thread_pool、async_logger、backend_worker 绑定
* 这能更灵活的自定义 async_logger 并复用 spdlog::thread_pool
* 由于 v2.x 还没有发行版,v1.x 拒绝合并破坏 api 的更改
  所以我们自己修改 spdlog 源码以满足自定义 async_logger 的需求
  后续需要特别注意此处的兼容性
* gabime/spdlog#3293
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants