fix: settings are read from db multiple times a second#420
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a caching mechanism for application settings to address performance issues where settings were being read from the database multiple times per second during UI repaints.
- Adds a
RwLock-protected settings cache toAppContextto reduce database reads - Implements cache invalidation with atomic updates using guard patterns
- Replaces direct database calls in UI components with
AppContextmethods
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/context.rs | Adds settings cache infrastructure with invalidation methods and cached retrieval |
| src/database/settings.rs | Adds documentation discouraging direct database method usage |
| src/ui/components/top_panel.rs | Updates to use cached settings via AppContext instead of direct DB calls |
| src/ui/network_chooser_screen.rs | Removes direct database access, uses AppContext method |
| src/ui/wallets/add_new_wallet_screen.rs | Removes direct database access for password updates |
| src/ui/wallets/import_wallet_screen.rs | Removes direct database access for password updates |
| src/backend_task/system_task/mod.rs | Adds cache invalidation for theme preference updates |
pauldelucia
left a comment
There was a problem hiding this comment.
One comment. But it fixes the issue and code looks good otherwise.
* fix: top up with qr code wasnt working (#410) * fix: top up with qr code wasnt working * fmt and clippy * coderabbit * fix: dash-qt path autodetection fails (#409) * fix: dash-qt path not detected correctly * fix: correct handling of dash-qt path clear button * chore: copilot review * fix: change of wallet on top up does not update the address (#414) * build(deps): upgrade build dependencies (egui 0.32.0, rusqlite 0.37, and others) (#406) * build(deps): update all rust dependencies * chore: replace depreacated ui.close_menu() * chore: upgrade Popup in identities screen * fix: actions menu items width too low * chore: all warnings fixed * chore: close identity actions on click outside --------- Co-authored-by: pauldelucia <pauldelucia2@gmail.com> * refactor: unified identity selector (#408) * refactor: unified identity selection * feat: identity selector implementation on multiple screens * feat: add label to IdentitySelector * chore: I think final * Update src/ui/components/identity_selector.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix: settings are read from db 20 times a second (#420) * feat: wallet unlock on wallet screen (#415) * refactor: Amount type and AmountInput component (#417) * feat: amount input, first building version * test(amount): fixed tests * chore: I think final * chore: my_tokens display correct amount * chore: transfer tokens update * chore: hide unit on rewards estimate column * chore: two new helper methods * chore: I think finals * cargo fmt * feat: component trait * impl Component for AmountInput * chore: updated component trait * chore: update for egui enabled state mgmt * doc: component design pattern doc * chore: component design pattern continued * chore: amount improvements * chore: copilot review * chore: amount improvements * backport: amount component from * chore: fix imports * chore: refactor * chore: futher refactor * chore: further refactor based on feedback * doc: simplified component design pattern description * chore: peer review * doc: update docs * chore: amount input * test: run tests using github actions and fix failing tests (#422) * fix: failing tests * gha: run tests * fix: tests fail due to lack of .env file * fix: incorrect decimals handling on token create, mint and burn screens (#419) * feat: amount input, first building version * test(amount): fixed tests * chore: I think final * chore: my_tokens display correct amount * chore: transfer tokens update * chore: hide unit on rewards estimate column * chore: two new helper methods * chore: I think finals * cargo fmt * feat: component trait * impl Component for AmountInput * chore: updated component trait * chore: update for egui enabled state mgmt * doc: component design pattern doc * chore: component design pattern continued * chore: amount improvements * chore: copilot review * chore: amount improvements * refactor: mint and burn token screens use AmountInput * chore: use AmountInput on token creator screen * fix: burn error handling * feat: errors displayed in the AmountInput component * fix: vertical align of amount input * backport: amount component from * chore: fix imports * chore: refactor * chore: futher refactor * chore: further refactor based on feedback * doc: simplified component design pattern description * chore: peer review * doc: update docs * chore: amount input * chore: fixes after merge * chore: self-review * feat: amout set decimal places + rename label => with_label * refactor: amount input init on token screen * chore: fix token creator layout * chore: format base amount with leading zeros in confirmation * chore: base supply 0 by default * feat: add network field to identity structures (#423) * feat: add network field to identity structures * fix * feat: add display for private key in both WIF and Hex formats (#424) * feat: add display for private key in both WIF and Hex formats * fix: display private keys as normal text and depend on color theme --------- Co-authored-by: pauldelucia <pauldelucia2@gmail.com> * feat: allow setting zmq uri in .env (#425) * feat: allow setting ZMQ URI * chore: rename zmq_endpoint to core_zmq_endpoint * fix: failing test in token screen needed update * fix: update CI to use rust version 1.88 to match rust-toolchain * feat: better display in key selector (#429) * refactor: unified confirmation dialog (#413) * refactor: unified alert window * chore: update CLAUDE to create reusable components whenever appropriate * feat: correct confirm dialog on set token price screen * chore: remove callbacks which are overkill * chore: cargo fmt * chore: doctest fix * chore: impl Component for ConfirmationDialog * chore: use WidgetText * feat: Add Escape key handling for confirmation dialog * chore: button inactive when in progress * chore: fixes after merge * chore: some theme improvements * fmt and visual appeal --------- Co-authored-by: pauldelucia <pauldelucia2@gmail.com> * feat: nicer contract chooser panel (#426) * feat: nicer contract chooser panel * fmt * clippy * fix: token purchasability was not refreshing unless app restart (#432) * fix: token action buttons alignment * feat: nicer expanding tabs in token creator (#431) * feat: nicer expanding tabs in token creator * fix * feat: implement new ConfirmationDialog component throughout app (#430) * feat: implement new ConfirmationDialog component throughout app * fix: remove backup files * fix: confirmation dialog on withdrawal screen always showing "loading" * feat: Dash Masternode List and Quorum Viewer (#428) * a lot of work on DML viewer * more work * more work * more work * more work * more work * more work * more work * more work * ui improvements * ui improvements * optimizations * fast start * more work * more work * more work * fmt * much more work * fixes * updates * fix * fmt * revert * update for dashcore 40 * params for testnet * added testnet diff * fixes * clippy * more clippy * fixes * clean UI * use backend tasks * backend messages * coderabbit suggestions to add timeouts and prevent infinite loops * transient and fatal errors * update dash core configs for testnet * fmt * fix timeout * fix h-3 error * clear state when switching networks --------- Co-authored-by: pauldelucia <pauldelucia2@gmail.com> * chore: bump version to 1.0.0-dev (#439) The 1.0-dev branch builds were still reporting as 0.9.0 * feat: add left panel button labels and create contract subscreen chooser panel (#441) * feat: add left panel button labels and create contract subscreen chooser panel * fmt * feat: clarify identity index description for identity creation * fix: screen button labels display hard to see in dark mode * feat: left panel scroll (#443) * feat: left panel scroll * clippy * feat: add existing identity by wallet + identity index (#444) * feat: add existing identity by wallet + identity index * fmt * remove logging * update derivation index label * add robustness * fix: screen button labels display hard to see in dark mode * feat: left panel scroll (#443) * feat: left panel scroll * clippy * feat: load all identities up to an index * clippy * fix * feat: remove wallet (#445) * feat: remove wallet * clippy * Clarify wallet removal warning message Updated warning message for wallet removal to clarify that keys will no longer work unless the wallet is re-imported. * fix: identity creation and top-up via QR code handling (#447) * fix: identity creation handling * cleanup * cleanup * fix identity topup too * remove note * fix: keyword search required new platform version plus error handling and UI updates (#449) * fix: keyword search not displaying errors * update platform version and keyword search cleanup * fix * fix: handle decimals and fix schedules on price and purchase screens (#412) * fix: inactive button on set price group action * chore: apply review feedback * refactor: use token amount input * chore: update AmountInput * chore: tiered pricing * chore: direct purchase * chore: remove total agreed price * chore: max amount input defaults to max_credits * fmt * fix * clippy --------- Co-authored-by: pauldelucia <pauldelucia2@gmail.com> Co-authored-by: Claude <noreply@anthropic.com> * fix: showing used asset locks as unused (#448) * chore: remove unused task sender * chore: update to Platform V10 * feat: GroveSTARK ZK proofs (#450) * works * works * fix * ok * ok * ok * ok * ok * fix verification message showing in generation screen * remove advanced settings. set default 16 grinding bits and 128 bit security * ok * lock * ok * clippy * fmt * clippy and fmt * pin grovestark dep * fix * ok * clippy * rename file * remove ProofData * cleanup * fix * rename GroveStark to GroveSTARK * rename zk_proofs_screen to grovestark_screen * rename ZKProofs tool subscreen to GroveSTARK * move grovestark prover to model * rename ContractsDashpayComingSoon to ContractsDashpay * rename ContractsDashpay to Dashpay * rename dashpay stuff * fixes * ok * update grovestark rev * update * chore: update grovestark rev * chore: bump version * update platform rev * clippy * fix * fix * fix release * fix release * fix release --------- Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: QuantumExplorer <quantum@dash.org> Co-authored-by: thephez <thephez@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
Problem statement
Settings are read from DB a few times with every repaint of UI.
Solution
This pull request introduces a caching mechanism for application settings to improve performance and ensure atomic updates. The key changes include adding a settings cache to
AppContext, implementing methods to manage the cache, and updating database interaction logic to integrate with the cache. Additionally, direct database calls in the UI layer have been replaced withAppContextmethods to ensure proper cache handling.Caching Mechanism Enhancements:
AppContext: Introduced aRwLock-protectedcached_settingsfield to store settings and allow concurrent reads while ensuring exclusive writes during updates (src/context.rs, [1] [2].invalidate_settings_cachemethod to clear the cache and return a guard to prevent race conditions during database updates (src/context.rs, src/context.rsR485-R551).get_settingsto first check the cache before querying the database, and to update the cache after a database read (src/context.rs, src/context.rsR485-R551).Database Interaction Updates:
AppContextmethods: Added documentation to database methods (e.g.,update_main_password,update_dash_core_execution_settings) to discourage direct calls and ensure proper caching behavior (src/database/settings.rs, [1] [2] [3] [4] [5].UI Layer Improvements:
AppContextmethods: Updated UI components to useAppContextmethods for settings updates and retrieval, ensuring cache consistency:add_connection_indicatornow usesget_settingsinstead of directly querying the database (src/ui/components/top_panel.rs, src/ui/components/top_panel.rsL162-R165).update_dash_core_execution_settingscalls inNetworkChooserScreennow useAppContext(src/ui/network_chooser_screen.rs, src/ui/network_chooser_screen.rsL123).AddNewWalletScreenandImportWalletScreennow useAppContext(src/ui/wallets/add_new_wallet_screen.rs, [1];src/ui/wallets/import_wallet_screen.rs, [2].