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

feature: Add support for dumping database contents #640

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

davidbarsky
Copy link
Contributor

@davidbarsky davidbarsky commented Dec 29, 2024

As discussed in this HackMD, we want to dump the contents of a Salsa database in order to debug what rust-analyzer and Salsa are doing. The original Salsa had a DebugQueryTable that we'd use like this:

let mut total_file_size = Bytes::default();
for e in ide_db::base_db::ParseQuery.in_db(db).entries::<Vec<_>>() {
    total_file_size += syntax_len(db.parse(e.key).syntax_node())
}

let mut total_macro_file_size = Bytes::default();
for e in hir::db::ParseMacroExpansionQuery.in_db(db).entries::<Vec<_>>() {
    let val = db.parse_macro_expansion(e.key).value.0;
    total_macro_file_size += syntax_len(val.syntax_node())
}
eprintln!("source files: {total_file_size}, macro files: {total_macro_file_size}");

(ParseQuery and ParseMacroExpansionQuery were generated from SourceDatabase::parse and ExpandDatabase::parse_macro_expansion, respectively.)

This PR takes a slightly different approach: instead of generating a Query struct corresponding to a tracked function or Salsa struct, this PR simply exposes the underlying pages on salsa::Storage via the debug_input_entries, debug_input_entries, and debug_interned_entries methods. This results in the following API:

let _ = InternedStruct::new(&db, "Hello, world!".to_string());

// test interned structs
let interned = db
    .storage()
    .debug_interned_entries::<InternedStruct>()
    .collect::<Vec<_>>();
    
assert_eq!(interned[0].data.0, "Hello, world!");

Few notes:

  • I don't see any obvious way to make these methods generic over the underlying Salsa struct, but I'm not sure it's desirable in the first place.
  • I'm not entirely happy with the name or API, so suggestions to improve it would be welcome. I don't know if I want to expose this debug information on the Salsa structs themselves, but I'd be happy to hear arguments in favor.
  • This approach requires exposing parts of {interned, tracked, input}::Value publicly, which feels suboptimal, but we're all friends here, right?

Copy link

netlify bot commented Dec 29, 2024

Deploy Preview for salsa-rs canceled.

Name Link
🔨 Latest commit 65f31e9
🔍 Latest deploy log https://app.netlify.com/sites/salsa-rs/deploys/6771984cc9d70a0008419d7a

Copy link

codspeed-hq bot commented Dec 29, 2024

CodSpeed Performance Report

Merging #640 will not alter performance

Comparing davidbarsky:davidbarsky/add-db-debug-introspection (65f31e9) with master (88a1d77)

Summary

✅ 9 untouched benchmarks

@davidbarsky davidbarsky marked this pull request as ready for review December 29, 2024 18:52
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.

1 participant