From 8b7f58976b32ccc768e35151b77aa9ea004b7495 Mon Sep 17 00:00:00 2001
From: Aleksey Kladov <aleksey.kladov@gmail.com>
Date: Sun, 2 Jun 2019 20:15:10 +0300
Subject: [PATCH] don't cache parses twice

Before this commit, `Parse`s for original file ended up two times in
salsa's db: first, when we parse original file, and second, when we
parse macro or a file.

Given that parse trees are the worst ofenders in terms of memory, it
makes sense to make sure we store them only once.
---
 crates/ra_hir/src/db.rs         |  3 ++
 crates/ra_hir/src/ids.rs        | 53 +++++++++++++++++----------------
 crates/ra_hir/src/lib.rs        |  2 +-
 crates/ra_ide_api/src/change.rs |  2 +-
 crates/ra_ide_api/src/status.rs | 34 +++++++++++++++++----
 5 files changed, 61 insertions(+), 33 deletions(-)

diff --git a/crates/ra_hir/src/db.rs b/crates/ra_hir/src/db.rs
index 87210321943..3afd0994ce8 100644
--- a/crates/ra_hir/src/db.rs
+++ b/crates/ra_hir/src/db.rs
@@ -33,8 +33,11 @@ pub trait AstDatabase: SourceDatabase {
     #[salsa::transparent]
     #[salsa::invoke(crate::source_id::AstIdMap::file_item_query)]
     fn ast_id_to_node(&self, file_id: HirFileId, ast_id: ErasedFileAstId) -> TreeArc<SyntaxNode>;
+    #[salsa::transparent]
     #[salsa::invoke(crate::ids::HirFileId::parse_or_expand_query)]
     fn parse_or_expand(&self, file_id: HirFileId) -> Option<TreeArc<SyntaxNode>>;
+    #[salsa::invoke(crate::ids::HirFileId::parse_macro_query)]
+    fn parse_macro(&self, macro_file: ids::MacroFile) -> Option<TreeArc<SyntaxNode>>;
 
     #[salsa::invoke(crate::ids::macro_def_query)]
     fn macro_def(&self, macro_id: MacroDefId) -> Option<Arc<mbe::MacroRules>>;
diff --git a/crates/ra_hir/src/ids.rs b/crates/ra_hir/src/ids.rs
index 79e32e579d8..a9556181212 100644
--- a/crates/ra_hir/src/ids.rs
+++ b/crates/ra_hir/src/ids.rs
@@ -62,32 +62,35 @@ impl HirFileId {
         file_id: HirFileId,
     ) -> Option<TreeArc<SyntaxNode>> {
         db.check_canceled();
-        let _p = profile("parse_or_expand_query");
         match file_id.0 {
             HirFileIdRepr::File(file_id) => Some(db.parse(file_id).tree.syntax().to_owned()),
-            HirFileIdRepr::Macro(macro_file) => {
-                let macro_call_id = macro_file.macro_call_id;
-                let tt = db
-                    .macro_expand(macro_call_id)
-                    .map_err(|err| {
-                        // Note:
-                        // The final goal we would like to make all parse_macro success,
-                        // such that the following log will not call anyway.
-                        log::warn!(
-                            "fail on macro_parse: (reason: {}) {}",
-                            err,
-                            macro_call_id.debug_dump(db)
-                        );
-                    })
-                    .ok()?;
-                match macro_file.macro_file_kind {
-                    MacroFileKind::Items => {
-                        Some(mbe::token_tree_to_ast_item_list(&tt).syntax().to_owned())
-                    }
-                    MacroFileKind::Expr => {
-                        mbe::token_tree_to_expr(&tt).ok().map(|it| it.syntax().to_owned())
-                    }
-                }
+            HirFileIdRepr::Macro(macro_file) => db.parse_macro(macro_file),
+        }
+    }
+
+    pub(crate) fn parse_macro_query(
+        db: &impl AstDatabase,
+        macro_file: MacroFile,
+    ) -> Option<TreeArc<SyntaxNode>> {
+        let _p = profile("parse_macro_query");
+        let macro_call_id = macro_file.macro_call_id;
+        let tt = db
+            .macro_expand(macro_call_id)
+            .map_err(|err| {
+                // Note:
+                // The final goal we would like to make all parse_macro success,
+                // such that the following log will not call anyway.
+                log::warn!(
+                    "fail on macro_parse: (reason: {}) {}",
+                    err,
+                    macro_call_id.debug_dump(db)
+                );
+            })
+            .ok()?;
+        match macro_file.macro_file_kind {
+            MacroFileKind::Items => Some(mbe::token_tree_to_ast_item_list(&tt).syntax().to_owned()),
+            MacroFileKind::Expr => {
+                mbe::token_tree_to_expr(&tt).ok().map(|it| it.syntax().to_owned())
             }
         }
     }
@@ -100,7 +103,7 @@ enum HirFileIdRepr {
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-struct MacroFile {
+pub struct MacroFile {
     macro_call_id: MacroCallId,
     macro_file_kind: MacroFileKind,
 }
diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs
index f2a31795df5..18dea5f1758 100644
--- a/crates/ra_hir/src/lib.rs
+++ b/crates/ra_hir/src/lib.rs
@@ -60,7 +60,7 @@ pub use self::{
     path::{Path, PathKind},
     name::Name,
     source_id::{AstIdMap, ErasedFileAstId},
-    ids::{HirFileId, MacroDefId, MacroCallId, MacroCallLoc},
+    ids::{HirFileId, MacroDefId, MacroCallId, MacroCallLoc, MacroFile},
     nameres::{PerNs, Namespace, ImportId},
     ty::{Ty, ApplicationTy, TypeCtor, TraitRef, Substs, display::HirDisplay, CallableDef},
     impl_block::{ImplBlock, ImplItem},
diff --git a/crates/ra_ide_api/src/change.rs b/crates/ra_ide_api/src/change.rs
index 02814ce5687..247dc0feed0 100644
--- a/crates/ra_ide_api/src/change.rs
+++ b/crates/ra_ide_api/src/change.rs
@@ -226,7 +226,7 @@ impl RootDatabase {
 
         self.query(ra_db::ParseQuery).sweep(sweep);
 
-        self.query(hir::db::ParseOrExpandQuery).sweep(sweep);
+        self.query(hir::db::ParseMacroQuery).sweep(sweep);
         self.query(hir::db::MacroDefQuery).sweep(sweep);
         self.query(hir::db::MacroArgQuery).sweep(sweep);
         self.query(hir::db::MacroExpandQuery).sweep(sweep);
diff --git a/crates/ra_ide_api/src/status.rs b/crates/ra_ide_api/src/status.rs
index 822279812c0..0cdeb15eb73 100644
--- a/crates/ra_ide_api/src/status.rs
+++ b/crates/ra_ide_api/src/status.rs
@@ -4,12 +4,12 @@ use std::{
     sync::Arc,
 };
 
-use ra_syntax::{TreeArc, SyntaxNode};
+use ra_syntax::{TreeArc, SyntaxNode, Parse, AstNode};
 use ra_db::{
     FileTextQuery, SourceRootId,
     salsa::{Database, debug::{DebugQueryTable, TableEntry}},
 };
-use hir::HirFileId;
+use hir::MacroFile;
 
 use crate::{
     FileId, db::RootDatabase,
@@ -17,18 +17,23 @@ use crate::{
 };
 
 pub(crate) fn syntax_tree_stats(db: &RootDatabase) -> SyntaxTreeStats {
-    db.query(hir::db::ParseOrExpandQuery).entries::<SyntaxTreeStats>()
+    db.query(ra_db::ParseQuery).entries::<SyntaxTreeStats>()
+}
+pub(crate) fn macro_syntax_tree_stats(db: &RootDatabase) -> SyntaxTreeStats {
+    db.query(hir::db::ParseMacroQuery).entries::<SyntaxTreeStats>()
 }
 
 pub(crate) fn status(db: &RootDatabase) -> String {
     let files_stats = db.query(FileTextQuery).entries::<FilesStats>();
     let syntax_tree_stats = syntax_tree_stats(db);
+    let macro_syntax_tree_stats = macro_syntax_tree_stats(db);
     let symbols_stats = db.query(LibrarySymbolsQuery).entries::<LibrarySymbolsStats>();
     format!(
-        "{}\n{}\n{}\n\n\nmemory:\n{}\ngc {:?} seconds ago",
+        "{}\n{}\n{}\n{} (macros)\n\n\nmemory:\n{}\ngc {:?} seconds ago",
         files_stats,
         symbols_stats,
         syntax_tree_stats,
+        macro_syntax_tree_stats,
         MemoryStats::current(),
         db.last_gc.elapsed().as_secs(),
     )
@@ -73,10 +78,27 @@ impl fmt::Display for SyntaxTreeStats {
     }
 }
 
-impl FromIterator<TableEntry<HirFileId, Option<TreeArc<SyntaxNode>>>> for SyntaxTreeStats {
+impl FromIterator<TableEntry<FileId, Parse>> for SyntaxTreeStats {
     fn from_iter<T>(iter: T) -> SyntaxTreeStats
     where
-        T: IntoIterator<Item = TableEntry<HirFileId, Option<TreeArc<SyntaxNode>>>>,
+        T: IntoIterator<Item = TableEntry<FileId, Parse>>,
+    {
+        let mut res = SyntaxTreeStats::default();
+        for entry in iter {
+            res.total += 1;
+            if let Some(tree) = entry.value.as_ref().map(|it| &it.tree) {
+                res.retained += 1;
+                res.retained_size += tree.syntax().memory_size_of_subtree();
+            }
+        }
+        res
+    }
+}
+
+impl FromIterator<TableEntry<MacroFile, Option<TreeArc<SyntaxNode>>>> for SyntaxTreeStats {
+    fn from_iter<T>(iter: T) -> SyntaxTreeStats
+    where
+        T: IntoIterator<Item = TableEntry<MacroFile, Option<TreeArc<SyntaxNode>>>>,
     {
         let mut res = SyntaxTreeStats::default();
         for entry in iter {