From 3beaf5f89d014526076335af2c8ac7fcff54d512 Mon Sep 17 00:00:00 2001 From: coolreader18 <33094578+coolreader18@users.noreply.github.com> Date: Wed, 10 Jul 2019 13:20:59 -0500 Subject: [PATCH] Address feedback Move profile writing to own function; move profiling doc to DEVELOPMENT.md, remove 'present because' from arg help. --- DEVELOPMENT.md | 56 +++++++++++++++++++++++++++++++ README.md | 52 ----------------------------- src/main.rs | 90 +++++++++++++++++++++++--------------------------- 3 files changed, 98 insertions(+), 100 deletions(-) create mode 100644 DEVELOPMENT.md diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md new file mode 100644 index 000000000..01e7fe1d6 --- /dev/null +++ b/DEVELOPMENT.md @@ -0,0 +1,56 @@ +# RustPython Development Guide and Tips + +## Code organization + +- `parser/src`: python lexing, parsing and ast +- `vm/src`: python virtual machine + - `builtins.rs`: Builtin functions + - `compile.rs`: the python compiler from ast to bytecode + - `obj`: python builtin types +- `src`: using the other subcrates to bring rustpython to life. +- `docs`: documentation (work in progress) +- `py_code_object`: CPython bytecode to rustpython bytecode converter (work in + progress) +- `wasm`: Binary crate and resources for WebAssembly build +- `tests`: integration test snippets + +## Code style + +The code style used is the default +[rustfmt](https://github.com/rust-lang/rustfmt) codestyle. Please format your +code accordingly. We also use [clippy](https://github.com/rust-lang/rust-clippy) +to detect rust code issues. + +## Testing + +To test rustpython, there is a collection of python snippets located in the +`tests/snippets` directory. To run those tests do the following: + +```shell +$ cd tests +$ pipenv install +$ pipenv run pytest -v +``` + +There also are some unit tests, you can run those with cargo: + +```shell +$ cargo test --all +``` + +## Profiling + +To profile rustpython, simply build in release mode with the `flame-it` feature. +This will generate a file `flamescope.json`, which you can then view at +https://speedscope.app. + +```sh +$ cargo run --release --features flame-it script.py +$ cat flamescope.json +{} +``` + +You can also pass the `--output-file` option to choose which file to output to +(or stdout if you specify `-`), and the `--output-format` option to choose if +you want to output in the speedscope json format (default), text, or a raw html +viewer (currently broken). diff --git a/README.md b/README.md index 01d93bcb3..9f110e1d3 100644 --- a/README.md +++ b/README.md @@ -59,19 +59,6 @@ Documentation HTML files can then be found in the `target/doc` directory. If you wish to update the online documentation, push directly to the `release` branch (or ask a maintainer to do so). This will trigger a Travis build that updates the documentation and WebAssembly demo page. -# Code organization - -- `parser/src`: python lexing, parsing and ast -- `vm/src`: python virtual machine - - `builtins.rs`: Builtin functions - - `compile.rs`: the python compiler from ast to bytecode - - `obj`: python builtin types -- `src`: using the other subcrates to bring rustpython to life. -- `docs`: documentation (work in progress) -- `py_code_object`: CPython bytecode to rustpython bytecode converter (work in progress) -- `wasm`: Binary crate and resources for WebAssembly build -- `tests`: integration test snippets - # Contributing Contributions are more than welcome, and in many cases we are happy to guide contributors through PRs or on gitter. @@ -88,40 +75,6 @@ You can also simply run `./whats_left.sh` to assist in finding any unimplemented method. -# Testing - -To test rustpython, there is a collection of python snippets located in the -`tests/snippets` directory. To run those tests do the following: - -```shell -$ cd tests -$ pipenv install -$ pipenv run pytest -v -``` - -There also are some unit tests, you can run those with cargo: - -```shell -$ cargo test --all -``` - -# Profiling - -To profile rustpython, simply build in release mode with the `flame-it` feature. -This will generate a file `flamescope.json`, which you can then view at -https://speedscope.app. - -```sh -$ cargo run --release --features flame-it script.py -$ cat flamescope.json -{} -``` - -You can also pass the `--output-file` option to choose which file to output to -(or stdout if you specify `-`), and the `--output-format` option to choose if -you want to output in the speedscope json format (default), text, or a raw html -viewer (currently broken). - # Using a standard library As of now the standard library is under construction. You can @@ -143,11 +96,6 @@ the [ouroboros library](https://github.com/pybee/ouroboros). [See this doc](wasm/README.md) -# Code style - -The code style used is the default [rustfmt](https://github.com/rust-lang/rustfmt) codestyle. Please format your code accordingly. -We also use [clippy](https://github.com/rust-lang/rust-clippy) to detect rust code issues. - # Community Chat with us on [gitter][gitter]. diff --git a/src/main.rs b/src/main.rs index 6fbb4c01a..477272323 100644 --- a/src/main.rs +++ b/src/main.rs @@ -22,13 +22,6 @@ use std::path::PathBuf; use std::process; fn main() { - if let Err(err) = run() { - error!("Error: {}", err); - process::exit(1); - } -} - -fn run() -> Result<(), Box> { #[cfg(feature = "flame-it")] let main_guard = flame::start_guard("RustPython main"); env_logger::init(); @@ -62,19 +55,13 @@ fn run() -> Result<(), Box> { Arg::with_name("profile_output") .long("profile-output") .takes_value(true) - .help( - "the file to output the profiling information to. present due to being \ - built with feature 'flame-it'", - ), + .help("the file to output the profiling information to"), ) .arg( Arg::with_name("profile_format") .long("profile-format") .takes_value(true) - .help( - "the profile format to output the profiling information in. present due to \ - being built with feature 'flame-it'", - ), + .help("the profile format to output the profiling information in"), ); let matches = app.get_matches(); @@ -102,46 +89,53 @@ fn run() -> Result<(), Box> { #[cfg(feature = "flame-it")] { - use std::fs::File; - main_guard.end(); - - enum ProfileFormat { - Html, - Text, - Speedscope, + if let Err(e) = write_profile(matches) { + error!("Error writing profile information: {}", e); + process::exit(1); } + } +} - let profile_output = matches.value_of_os("profile_output"); +#[cfg(feature = "flame-it")] +fn write_profile(matches: clap::ArgMatches) -> Result<(), Box> { + use std::fs::File; - let profile_format = match matches.value_of("profile_format") { - Some("html") => ProfileFormat::Html, - Some("text") => ProfileFormat::Text, - None if profile_output == Some("-".as_ref()) => ProfileFormat::Text, - Some("speedscope") | None => ProfileFormat::Speedscope, - Some(other) => { - error!("Unknown profile format {}", other); - process::exit(1); - } - }; + enum ProfileFormat { + Html, + Text, + Speedscope, + } - let profile_output = profile_output.unwrap_or_else(|| match profile_format { - ProfileFormat::Html => "flame-graph.html".as_ref(), - ProfileFormat::Text => "flame.txt".as_ref(), - ProfileFormat::Speedscope => "flamescope.json".as_ref(), - }); + let profile_output = matches.value_of_os("profile_output"); - let profile_output: Box = if profile_output == "-" { - Box::new(std::io::stdout()) - } else { - Box::new(File::create(profile_output)?) - }; - - match profile_format { - ProfileFormat::Html => flame::dump_html(profile_output)?, - ProfileFormat::Text => flame::dump_text_to_writer(profile_output)?, - ProfileFormat::Speedscope => flamescope::dump(profile_output)?, + let profile_format = match matches.value_of("profile_format") { + Some("html") => ProfileFormat::Html, + Some("text") => ProfileFormat::Text, + None if profile_output == Some("-".as_ref()) => ProfileFormat::Text, + Some("speedscope") | None => ProfileFormat::Speedscope, + Some(other) => { + error!("Unknown profile format {}", other); + process::exit(1); } + }; + + let profile_output = profile_output.unwrap_or_else(|| match profile_format { + ProfileFormat::Html => "flame-graph.html".as_ref(), + ProfileFormat::Text => "flame.txt".as_ref(), + ProfileFormat::Speedscope => "flamescope.json".as_ref(), + }); + + let profile_output: Box = if profile_output == "-" { + Box::new(std::io::stdout()) + } else { + Box::new(File::create(profile_output)?) + }; + + match profile_format { + ProfileFormat::Html => flame::dump_html(profile_output)?, + ProfileFormat::Text => flame::dump_text_to_writer(profile_output)?, + ProfileFormat::Speedscope => flamescope::dump(profile_output)?, } Ok(())