Skip to content

Commit

Permalink
[move-compiler] Parallelize parsing files. Parallelize visitors. (Mys…
Browse files Browse the repository at this point in the history
…tenLabs#20097)

## Description 

- Made parsing parallel
- Made visitor execution parallel
- Unfortunately, even the sui-framework is not large enough to really
notice a difference (if anything it got a bit slower)

## Test plan 

- tests

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
- [ ] REST API:
  • Loading branch information
tnowacki authored Nov 1, 2024
1 parent d2a9fb6 commit c36e59d
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 48 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions external-crates/move/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion external-crates/move/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ proptest = "1.0.0"
proptest-derive = "0.3.0"
quote = "1.0.9"
rand = "0.8.0"
rayon = "1.5.0"
rayon = "1.10.0"
ref-cast = "1.0.6"
regex = "1.5.5"
ripemd160 = "0.9.1"
Expand Down
2 changes: 2 additions & 0 deletions external-crates/move/crates/move-compiler/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ petgraph.workspace = true
tempfile.workspace = true
once_cell.workspace = true
lsp-types.workspace = true
rayon.workspace = true
serde.workspace = true
serde_json.workspace = true
similar.workspace = true
stacker.workspace = true
vfs.workspace = true


bcs.workspace = true

move-binary-format.workspace = true
Expand Down
20 changes: 12 additions & 8 deletions external-crates/move/crates/move-compiler/src/cfgir/translate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use petgraph::{
algo::{kosaraju_scc as petgraph_scc, toposort as petgraph_toposort},
graphmap::DiGraphMap,
};
use rayon::prelude::*;
use std::{
collections::{BTreeMap, BTreeSet, VecDeque},
sync::Arc,
Expand Down Expand Up @@ -996,9 +997,12 @@ fn visit_program(context: &mut Context, prog: &mut G::Program) {

AbsintVisitor.visit(context.env, prog);

for v in &context.env.visitors().cfgir {
v.visit(context.env, prog)
}
context
.env
.visitors()
.cfgir
.par_iter()
.for_each(|v| v.visit(context.env, prog));
}

struct AbsintVisitor;
Expand Down Expand Up @@ -1088,11 +1092,11 @@ impl<'a> CFGIRVisitorContext for AbsintVisitorContext<'a> {
locals,
infinite_loop_starts: &infinite_loop_starts,
};
let mut ds = Diagnostics::new();
for v in &self.env.visitors().abs_int {
ds.extend(v.verify(&function_context, &cfg));
}
self.add_diags(ds);
self.env
.visitors()
.abs_int
.par_iter()
.for_each(|v| self.add_diags(v.verify(&function_context, &cfg)));
true
}
}
110 changes: 75 additions & 35 deletions external-crates/move/crates/move-compiler/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,34 @@ pub(crate) mod verification_attribute_filter;

use crate::{
parser::{self, ast::PackageDefinition, syntax::parse_file_string},
shared::{files::MappedFiles, CompilationEnv, IndexedVfsPackagePath, NamedAddressMaps},
shared::{
files::MappedFiles, CompilationEnv, IndexedVfsPackagePath, NamedAddressMapIndex,
NamedAddressMaps,
},
};
use anyhow::anyhow;
use comments::*;
use move_command_line_common::files::FileHash;
use move_symbol_pool::Symbol;
use rayon::iter::*;
use std::{collections::BTreeSet, sync::Arc};
use vfs::VfsPath;

struct ParsedFile {
fname: Symbol,
defs: Vec<parser::ast::Definition>,
comments: MatchedFileCommentMap,
hash: FileHash,
text: Arc<str>,
}

struct ParsedPackageFile {
is_dep: bool,
package: Option<Symbol>,
named_address_map: NamedAddressMapIndex,
file: ParsedFile,
}

/// Parses program's targets and dependencies, both of which are read from different virtual file
/// systems (vfs and deps_out_vfs, respectively).
pub(crate) fn parse_program(
Expand All @@ -40,35 +59,51 @@ pub(crate) fn parse_program(
let mut source_comments = CommentMap::new();
let mut lib_definitions = Vec::new();

for IndexedVfsPackagePath {
package,
path,
named_address_map,
} in targets
{
let (defs, comments, file_hash) = parse_file(&path, compilation_env, &mut files, package)?;
source_definitions.extend(defs.into_iter().map(|def| PackageDefinition {
let parsed = targets
.into_par_iter()
.map(|p| (false, p))
.chain(deps.into_par_iter().map(|p| (true, p)))
.map(|(is_dep, package_path)| {
let IndexedVfsPackagePath {
package,
path,
named_address_map,
} = package_path;
let file = parse_file(&path, compilation_env, package)?;
Ok(ParsedPackageFile {
is_dep,
package,
named_address_map,
file,
})
})
.collect::<anyhow::Result<Vec<_>>>()?;
for parsed_package_file in parsed {
let ParsedPackageFile {
is_dep,
package,
named_address_map,
def,
}));
source_comments.insert(file_hash, comments);
}

for IndexedVfsPackagePath {
package,
path,
named_address_map,
} in deps
{
let (defs, dep_comment_map, fhash) =
parse_file(&path, compilation_env, &mut files, package)?;
lib_definitions.extend(defs.into_iter().map(|def| PackageDefinition {
file,
} = parsed_package_file;
let ParsedFile {
fname,
defs,
comments,
hash,
text,
} = file;
files.add(hash, fname, text);
source_comments.insert(hash, comments);
let defs = defs.into_iter().map(|def| PackageDefinition {
package,
named_address_map,
def,
}));
source_comments.insert(fhash, dep_comment_map);
});
if is_dep {
lib_definitions.extend(defs);
} else {
source_definitions.extend(defs)
}
}

let pprog = parser::ast::Program {
Expand Down Expand Up @@ -114,13 +149,8 @@ fn ensure_targets_deps_dont_intersect(
fn parse_file(
path: &VfsPath,
compilation_env: &CompilationEnv,
files: &mut MappedFiles,
package: Option<Symbol>,
) -> anyhow::Result<(
Vec<parser::ast::Definition>,
MatchedFileCommentMap,
FileHash,
)> {
) -> anyhow::Result<ParsedFile> {
let mut source_buffer = String::new();
path.open_file()?.read_to_string(&mut source_buffer)?;
let file_hash = FileHash::new(&source_buffer);
Expand All @@ -129,8 +159,13 @@ fn parse_file(
let reporter = compilation_env.diagnostic_reporter_at_top_level();
if let Err(ds) = verify_string(file_hash, &source_str) {
reporter.add_diags(ds);
files.add(file_hash, fname, source_str);
return Ok((vec![], MatchedFileCommentMap::new(), file_hash));
return Ok(ParsedFile {
fname,
defs: vec![],
comments: MatchedFileCommentMap::new(),
hash: file_hash,
text: source_str,
});
}
let (defs, comments) = match parse_file_string(compilation_env, file_hash, &source_str, package)
{
Expand All @@ -140,6 +175,11 @@ fn parse_file(
(vec![], MatchedFileCommentMap::new())
}
};
files.add(file_hash, fname, source_str);
Ok((defs, comments, file_hash))
Ok(ParsedFile {
fname,
defs,
comments,
hash: file_hash,
text: source_str,
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -4602,7 +4602,7 @@ fn consume_spec_string(context: &mut Context) -> Result<Spanned<String>, Box<Dia
));
}

s.push_str(dbg!(context.tokens.content()));
s.push_str(context.tokens.content());
context.tokens.advance()?;

let mut count = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ use crate::{
};
use move_ir_types::location::*;
use move_proc_macros::growing_stack;
use rayon::prelude::*;
use std::{
collections::{BTreeMap, BTreeSet, VecDeque},
sync::Arc,
Expand Down Expand Up @@ -87,9 +88,11 @@ pub fn program(
modules,
info: Arc::new(module_info),
};
for v in &compilation_env.visitors().typing {
v.visit(compilation_env, &prog);
}
compilation_env
.visitors()
.typing
.par_iter()
.for_each(|v| v.visit(compilation_env, &prog));
prog
}

Expand Down

0 comments on commit c36e59d

Please sign in to comment.