logseq#12539: reduces memory allocation for graph file query to avoid crash; adds debug output - #4
Open
vr-one wants to merge 1 commit into
Open
logseq#12539: reduces memory allocation for graph file query to avoid crash; adds debug output#4vr-one wants to merge 1 commit into
vr-one wants to merge 1 commit into
Conversation
… crash; adds debug output
Author
|
@andelf @tiensonqin @RCmerci Please review this PR to build a new version of the raspi library and release a maintenance version of logseq - cause the db-version still lacks features to switch for most users. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix OOM crash in
get_all_files_meta+ improve error messages and loggingProblem
Logseq Desktop would crash with
EXC_BREAKPOINT (SIGTRAP)on thetokio-runtime-workerthread during file sync on large graphs. The stack trace pointed to an allocation failure insideget_file_metaat graph.rs.Root cause:
get_all_files_metacollects oneget_file_metafuture per file and passes them all tofuture::join_all, which polls every future concurrently. Each future immediately allocated aVec::with_capacity(1024 * 1024)(1 MB) read buffer and held it live for the duration of the I/O loop. With hundreds of files this meant hundreds of MB were reserved simultaneously, exhausting available memory and triggering a Rust OOM panic — which on ARM64 manifests asEXC_BREAKPOINT.Changes
graph.rs — core fix + diagnostics
get_file_metafrom 1 MB to 64 KB. The buffer is reset tolen = 0after each chunk, so only capacity is relevant to memory use; 64 KB is more than sufficient for efficient I/O and cuts peak concurrent allocation by 16×.unwrap()calls inis_page_file_pathwithunwrap_or_else(|| panic!(...))that include the offending path, making future panics immediately actionable.log::trace!at the entry ofget_file_metato aid per-file debugging.lib.rs — observability
get_local_all_files_meta: added a wall-clock timer; logs file count and elapsed milliseconds atINFOlevel after each full scan (e.g."get file meta of "/path/to/graph": 347 files in 412ms").get_local_files_meta: addedlog::trace!for the file list being requested.rename_local_file: addedlog::info!showing source and destination paths.sync.rs — panic quality
unwrap()calls with.expect(…)orunwrap_or_else(|| panic!(…))carrying descriptive context:Proxy::https(proxy)— identifies invalidHTTPS_PROXYURLbuilder.build()— identifies HTTP client construction failurestrip_prefix(bucket())— shows the mismatched prefix and bucket valuesself.credentials.as_ref()inupload_tempfile— clarifies thatrefresh_temp_credentialwas not calledself.s3_prefix.clone()— sameTesting
Verified on macOS arm64 with a ~350-file graph that previously triggered the crash. The app now completes the initial file-sync scan without crashing. The
INFOlog line confirms the correct file count and a reasonable scan time.