-
Notifications
You must be signed in to change notification settings - Fork 37.7k
multiprocess: Delay wallet client construction #23005
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
Conversation
Delay wallet client construction until after logging, thread and other init for two reasons: - More responsive multiprocess GUI startup. When bitcoin-gui is started this moves the call from bitcoin-gui to bitcoin-node that spawns bitcoin-wallet off of the GUI event thread and onto the background GUI init executor thread. - Avoids feature_logging.py test failures with bitcoin-node by making bitcoin-wallet logging start after bitcoin-node logging starts, because the tests are not written to handle the bitcoin-wallet logging init code running first. This partially reverts commit b266b3e, moving wallet client creation back to the place it was located before.
ca0704c
to
ad085f9
Compare
Updated ca0704c -> ad085f9 ( |
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK ad085f9, I have reviewed the code and it looks OK.
Also slightly tested, including the GUI: can observe wallet init messages on the splash screen, as expected.
code review ACK ad085f9 |
@@ -82,6 +82,9 @@ class CClientUIInterface | |||
/** Progress message during initialization. */ | |||
ADD_SIGNALS_DECL_WRAPPER(InitMessage, void, const std::string& message); | |||
|
|||
/** Wallet client created. */ | |||
ADD_SIGNALS_DECL_WRAPPER(InitWallet, void, ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason for the trailing comma?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason for the trailing comma?
This is to fix the clang warning ./node/ui_interface.h:86:46: error: must specify at least one argument for '...' parameter of variadic macro [-Werror,-Wgnu-zero-variadic-macro-arguments] ADD_SIGNALS_DECL_WRAPPER(InitWallet, void);
https://cirrus-ci.com/task/6303923532201984
Delay wallet client construction until after logging, thread and other init for two reasons:
More responsive multiprocess GUI startup. When bitcoin-gui is started this moves the call from bitcoin-gui to bitcoin-node that spawns bitcoin-wallet off of the GUI event thread and onto the background GUI init executor thread.
Avoids feature_logging.py test failures with bitcoin-node by making bitcoin-wallet logging start after bitcoin-node logging starts,
because the tests are not written to handle the bitcoin-wallet logging init code running first.
This partially reverts commit b266b3e, moving wallet client creation back to the place it was located before.
This PR is part of the process separation project.