fix(fluxdoc): use db when generating docs

Previously the fluxdoc gen would run a compilation step using `fluxc` to
speed up compilation. Compilation is very fast now and we have the salsa
db that does proper caching of intermediate steps. This change removes
the `fluxc` binary and the left over unused code. Also we update the
`fluxdoc` binary to use the salsa db only.

Since this is potentially a perf change I tested how fast the docs
generate, its dominated almost exclusively in running the examples.
Compiling the stdlib is instantaneous in my system.

Author: nathanielc

Makefile

@@ -191,17 +191,15 @@ bin/flux: $(STDLIB_SOURCES) $$(call go_deps,./cmd/flux)
 	$(GO_BUILD) -o ./bin/flux ./cmd/flux
 
 
-libflux/target/release/fluxc: libflux
-	cd libflux && $(CARGO) build $(CARGO_ARGS) --release --bin fluxc
 
 libflux/target/release/fluxdoc: libflux
 	cd libflux && $(CARGO) build $(CARGO_ARGS) --features=doc --release --bin fluxdoc
 
-fluxdocs: $(STDLIB_SOURCES) libflux/target/release/fluxc libflux/target/release/fluxdoc bin/flux
-	FLUXC=./libflux/target/release/fluxc FLUXDOC=./libflux/target/release/fluxdoc ./etc/gen_docs.sh
+fluxdocs: $(STDLIB_SOURCES) libflux/target/release/fluxdoc bin/flux
+	FLUXDOC=./libflux/target/release/fluxdoc ./etc/gen_docs.sh
 
-checkdocs: $(STDLIB_SOURCES) libflux/target/release/fluxc libflux/target/release/fluxdoc bin/flux
-	FLUXC=./libflux/target/release/fluxc FLUXDOC=./libflux/target/release/fluxdoc ./etc/checkdocs.sh
+checkdocs: $(STDLIB_SOURCES) libflux/target/release/fluxdoc bin/flux
+	FLUXDOC=./libflux/target/release/fluxdoc ./etc/checkdocs.sh
 
 # This list is sorted for easy inspection
 .PHONY: bench \

etc/checkdocs.sh

@@ -3,8 +3,7 @@
 set -e
 
 stdlib=${FLUX_STDLIB_DIR-./stdlib}
-fluxc=${FLUXC-fluxc}
 fluxdoc=${FLUXDOC-fluxdoc}
 
 # Lint the docs to ensure they are valid
-$fluxdoc lint --dir ${stdlib} --stdlib-dir "${stdlib}"
+$fluxdoc lint --dir ${stdlib}

etc/gen_docs.sh

@@ -3,22 +3,13 @@
 set -e
 
 stdlib=${FLUX_STDLIB_SRC-./stdlib}
-fluxc=${FLUXC-fluxc}
 fluxdoc=${FLUXDOC-fluxdoc}
 gendir=${DOCS_GEN_DIR-./generated_docs}
 full_docs="${gendir}/flux-docs-full.json"
 short_docs="${gendir}/flux-docs-short.json"
 
-dir=$(mktemp -d)
-stdlib_compiled="${dir}/stdlib"
-
-# Compile stdlib to a temporary dir
-mkdir -p $stdlib_compiled
-$fluxc stdlib --srcdir "${stdlib}" --outdir "${stdlib_compiled}"
-
 # Generate docs JSON files
 mkdir -p "${gendir}"
-$fluxdoc dump --dir ${stdlib} --stdlib-dir "${stdlib_compiled}" --output ${full_docs} --nested
-$fluxdoc dump --dir ${stdlib} --stdlib-dir "${stdlib_compiled}" --output ${short_docs} --short
+$fluxdoc dump --dir ${stdlib} --output ${full_docs} --nested
+$fluxdoc dump --dir ${stdlib} --output ${short_docs} --short
 
-rm -rf "${dir}"

libflux/flux-core/Cargo.toml

@@ -20,10 +20,6 @@ test = false
 bench = false
 required-features = ["doc"]
 
-[[bin]]
-name = "fluxc"
-test = false
-bench = false
 
 [features]
 default = ["strict"]

libflux/flux-core/src/bin/fluxc.rs

@@ -21,9 +21,6 @@ use fluxcore::{
 enum FluxDoc {
     /// Dump JSON encoding of documentation from Flux source code.
     Dump {
-        /// Directory containing Flux source code.
-        #[structopt(short, long, parse(from_os_str))]
-        stdlib_dir: Option<PathBuf>,
         /// Path to flux command, must be the cmd from internal/cmd/flux
         #[structopt(long, parse(from_os_str))]
         flux_cmd_path: Option<PathBuf>,
@@ -42,9 +39,6 @@ enum FluxDoc {
     },
     /// Check Flux source code for documentation linting errors
     Lint {
-        /// Directory containing Flux source code.
-        #[structopt(short, long, parse(from_os_str))]
-        stdlib_dir: Option<PathBuf>,
         /// Path to flux command, must be the cmd from internal/cmd/flux
         #[structopt(long, parse(from_os_str))]
         flux_cmd_path: Option<PathBuf>,
@@ -63,64 +57,52 @@ fn main() -> Result<()> {
     let app = FluxDoc::from_args();
     match app {
         FluxDoc::Dump {
-            stdlib_dir,
             flux_cmd_path,
             dir,
             output,
             nested,
             short,
         } => dump(
-            stdlib_dir.as_deref(),
             flux_cmd_path.as_deref(),
             &dir,
             output.as_deref(),
             nested,
             short,
         )?,
         FluxDoc::Lint {
-            stdlib_dir,
             flux_cmd_path,
             dir,
             limit,
-        } => lint(stdlib_dir.as_deref(), flux_cmd_path.as_deref(), &dir, limit)?,
+        } => lint(flux_cmd_path.as_deref(), &dir, limit)?,
     };
     Ok(())
 }
 
-const DEFAULT_STDLIB_PATH: &str = "./stdlib-compiled";
 const DEFAULT_FLUX_CMD_PATH: &str = "flux";
 
-fn resolve_default_paths<'a>(
-    stdlib_dir: Option<&'a Path>,
-    flux_cmd_path: Option<&'a Path>,
-) -> (&'a Path, &'a Path) {
-    let stdlib_dir = match stdlib_dir {
-        Some(stdlib_dir) => stdlib_dir,
-        None => Path::new(DEFAULT_STDLIB_PATH),
-    };
+fn resolve_default_paths<'a>(flux_cmd_path: Option<&'a Path>) -> &'a Path {
     let flux_cmd_path = match flux_cmd_path {
         Some(flux_cmd_path) => flux_cmd_path,
         None => Path::new(DEFAULT_FLUX_CMD_PATH),
     };
-    (stdlib_dir, flux_cmd_path)
+    flux_cmd_path
 }
 
 fn dump(
-    stdlib_dir: Option<&Path>,
     flux_cmd_path: Option<&Path>,
     dir: &Path,
     output: Option<&Path>,
     nested: bool,
     short: bool,
 ) -> Result<()> {
-    let (stdlib_dir, flux_cmd_path) = resolve_default_paths(stdlib_dir, flux_cmd_path);
+    let flux_cmd_path = resolve_default_paths(flux_cmd_path);
     let f = match output {
         Some(p) => Box::new(File::create(p).context(format!("creating output file {:?}", p))?)
             as Box<dyn io::Write>,
         None => Box::new(io::stdout()),
     };
 
-    let (mut docs, diagnostics) = parse_docs(stdlib_dir, dir).context("parsing source code")?;
+    let (mut docs, diagnostics) = parse_docs(dir).context("parsing source code")?;
     if !diagnostics.is_empty() {
         bail!(
             "found {} diagnostics when building documentation:\n{}",
@@ -157,19 +139,14 @@ fn dump(
     Ok(())
 }
 
-fn lint(
-    stdlib_dir: Option<&Path>,
-    flux_cmd_path: Option<&Path>,
-    dir: &Path,
-    limit: Option<i64>,
-) -> Result<()> {
-    let (stdlib_dir, flux_cmd_path) = resolve_default_paths(stdlib_dir, flux_cmd_path);
+fn lint(flux_cmd_path: Option<&Path>, dir: &Path, limit: Option<i64>) -> Result<()> {
+    let flux_cmd_path = resolve_default_paths(flux_cmd_path);
     let limit = match limit {
         Some(limit) if limit == 0 => i64::MAX,
         Some(limit) => limit,
         None => 10,
     };
-    let (mut docs, mut diagnostics) = parse_docs(stdlib_dir, dir)?;
+    let (mut docs, mut diagnostics) = parse_docs(dir)?;
     let mut pass = true;
     if !diagnostics.is_empty() {
         let rest = diagnostics.len() as i64 - limit;
@@ -198,7 +175,9 @@ fn lint(
         for result in results {
             test_count += 1;
             match result {
-                Ok(name) => eprintln!("OK ... {}", name),
+                Ok((name, duration)) => {
+                    eprintln!("OK ... {}, took {}ms", name, duration.as_millis())
+                }
                 Err(e) => {
                     eprintln!("Error {:?}\n", e);
                     pass = false;
@@ -271,14 +250,13 @@ fn consume_sequentially<T>(
 }
 
 /// Parse documentation for the specified directory.
-fn parse_docs(stdlib_dir: &Path, dir: &Path) -> Result<(Vec<doc::PackageDoc>, doc::Diagnostics)> {
+fn parse_docs(dir: &Path) -> Result<(Vec<doc::PackageDoc>, doc::Diagnostics)> {
     let db = DatabaseBuilder::default()
-        // We resolve paths in stdlib_dir first, then `dir` which mimicks the previous behavior
-        // most closely
-        .filesystem_roots(vec![stdlib_dir.into(), dir.into()])
+        .filesystem_roots(vec![dir.into()])
         .build();
 
-    let package_names = bootstrap::parse_dir(dir)?;
+    let mut package_names = bootstrap::parse_dir(dir)?;
+    package_names.sort();
     let mut docs = Vec::with_capacity(package_names.len());
     let mut diagnostics = Vec::new();
     for pkgpath in package_names {

libflux/flux-core/src/bin/fluxdoc.rs

@@ -1,7 +1,11 @@
 //! Parse documentation examples for their code and execute them collecting their inputs and
 //! outputs.
 
-use std::{collections::HashMap, ops::Range};
+use std::{
+    collections::HashMap,
+    ops::Range,
+    time::{Duration, SystemTime},
+};
 
 use anyhow::{bail, Context, Result};
 use csv::StringRecord;
@@ -20,7 +24,7 @@ pub trait Executor {
 }
 
 /// A list of test results
-pub type TestResults = Vec<Result<String>>;
+pub type TestResults = Vec<Result<(String, Duration)>>;
 
 /// Evaluates all examples in the documentation perfoming an inplace update of their inputs/outpts
 /// and content.
@@ -31,7 +35,7 @@ pub fn evaluate_package_examples(docs: &mut PackageDoc, executor: &impl Executor
         results.push(
             evaluate_example(example, executor)
                 .with_context(|| format!("executing example for package {}", path))
-                .map(|()| path.to_string()),
+                .map(|duration| (path.to_string(), duration)),
         );
     }
     for (name, doc) in docs.members.iter_mut() {
@@ -41,7 +45,7 @@ pub fn evaluate_package_examples(docs: &mut PackageDoc, executor: &impl Executor
                 .map(|result| {
                     result
                         .with_context(|| format!("executing example for {}.{}", path, name))
-                        .map(|title| format!("{}.{}.{}", path, name, title))
+                        .map(|(title, duration)| (format!("{}.{}:{}", path, name, title), duration))
                 }),
         );
     }
@@ -55,21 +59,27 @@ fn evaluate_doc_examples(doc: &mut Doc, executor: &impl Executor) -> TestResults
         Doc::Value(v) => v
             .examples
             .iter_mut()
-            .map(|example| evaluate_example(example, executor).map(|()| example.title.clone()))
+            .map(|example| {
+                evaluate_example(example, executor)
+                    .with_context(|| format!("executing `{}`", example.title))
+                    .map(|duration| (example.title.clone(), duration))
+            })
             .collect(),
         Doc::Function(f) => f
             .examples
             .iter_mut()
             .map(|example| {
                 evaluate_example(example, executor)
                     .with_context(|| format!("executing `{}`", example.title))
-                    .map(|()| example.title.clone())
+                    .map(|duration| (example.title.clone(), duration))
             })
             .collect(),
     }
 }
 
-fn evaluate_example(example: &mut Example, executor: &impl Executor) -> Result<()> {
+fn evaluate_example(example: &mut Example, executor: &impl Executor) -> Result<Duration> {
+    // Time the evaluation
+    let start = SystemTime::now();
     let blocks = preprocess_code_blocks(&example.content)?;
     if blocks.len() > 1 {
         bail!(
@@ -89,7 +99,8 @@ fn evaluate_example(example: &mut Example, executor: &impl Executor) -> Result<(
             BlockMode::NoRun => {}
         }
     }
-    Ok(())
+    let end = SystemTime::now();
+    Ok(end.duration_since(start)?)
 }
 
 enum BlockMode {

libflux/flux-core/src/doc/example.rs

@@ -3,17 +3,15 @@
 //! This package does not assume a location of the source code but does assume which packages are
 //! part of the prelude.
 
-use std::{env::consts, fs, io, io::Write, path::Path, sync::Arc};
+use std::{env::consts, io, path::Path, sync::Arc};
 
 use anyhow::{bail, Result};
-use libflate::gzip::Encoder;
 use walkdir::WalkDir;
 
 use crate::{
     ast,
     db::{DatabaseBuilder, Flux},
     semantic::{
-        flatbuffers::types::{build_module, finish_serialize},
         fs::{FileSystemImporter, StdFS},
         import::{Importer, Packages},
         nodes::{self, Package, Symbol},
@@ -202,35 +200,6 @@ pub fn stdlib(dir: &Path) -> Result<(PackageExports, FileSystemImporter<StdFS>)>
     Ok((prelude, stdlib_importer))
 }
 
-/// Compiles the stdlib found at the srcdir into the outdir.
-pub fn compile_stdlib(srcdir: &Path, outdir: &Path) -> Result<()> {
-    let (_, imports, mut sem_pkgs) = infer_stdlib_dir(srcdir, AnalyzerConfig::default())?;
-    // Write each file as compiled module
-    for (path, exports) in &imports {
-        if let Some(code) = sem_pkgs.remove(path) {
-            let module = Module {
-                polytype: Some(exports.typ()),
-                code: Some(code),
-            };
-            let mut builder = flatbuffers::FlatBufferBuilder::new();
-            let offset = build_module(&mut builder, module);
-            let buf = finish_serialize(&mut builder, offset);
-
-            // Write module contents to file
-            let mut fpath = outdir.join(path);
-            fpath.set_extension("fc");
-            fs::create_dir_all(fpath.parent().unwrap())?;
-            let file = fs::File::create(&fpath)?;
-            let mut encoder = Encoder::new(file)?;
-            encoder.write_all(buf)?;
-            encoder.finish().into_result()?;
-        } else {
-            bail!("package {} missing code", &path);
-        }
-    }
-    Ok(())
-}
-
 /// Module represenets the result of compiling Flux source code.
 ///
 /// The polytype represents the type of the entire package as a record type.