Skip to content

Commit 3beaf5f

Browse files
coolreader18coolreader18
committed
Address feedback
Move profile writing to own function; move profiling doc to DEVELOPMENT.md, remove 'present because' from arg help.
1 parent 8e06a30 commit 3beaf5f

File tree

3 files changed

+98
-100
lines changed

3 files changed

+98
-100
lines changed

‎DEVELOPMENT.md

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
# RustPython Development Guide and Tips
2+
3+
## Code organization
4+
5+
- `parser/src`: python lexing, parsing and ast
6+
- `vm/src`: python virtual machine
7+
- `builtins.rs`: Builtin functions
8+
- `compile.rs`: the python compiler from ast to bytecode
9+
- `obj`: python builtin types
10+
- `src`: using the other subcrates to bring rustpython to life.
11+
- `docs`: documentation (work in progress)
12+
- `py_code_object`: CPython bytecode to rustpython bytecode converter (work in
13+
progress)
14+
- `wasm`: Binary crate and resources for WebAssembly build
15+
- `tests`: integration test snippets
16+
17+
## Code style
18+
19+
The code style used is the default
20+
[rustfmt](https://github.com/rust-lang/rustfmt) codestyle. Please format your
21+
code accordingly. We also use [clippy](https://github.com/rust-lang/rust-clippy)
22+
to detect rust code issues.
23+
24+
## Testing
25+
26+
To test rustpython, there is a collection of python snippets located in the
27+
`tests/snippets` directory. To run those tests do the following:
28+
29+
```shell
30+
$ cd tests
31+
$ pipenv install
32+
$ pipenv run pytest -v
33+
```
34+
35+
There also are some unit tests, you can run those with cargo:
36+
37+
```shell
38+
$ cargo test --all
39+
```
40+
41+
## Profiling
42+
43+
To profile rustpython, simply build in release mode with the `flame-it` feature.
44+
This will generate a file `flamescope.json`, which you can then view at
45+
https://speedscope.app.
46+
47+
```sh
48+
$ cargo run --release --features flame-it script.py
49+
$ cat flamescope.json
50+
{<json>}
51+
```
52+
53+
You can also pass the `--output-file` option to choose which file to output to
54+
(or stdout if you specify `-`), and the `--output-format` option to choose if
55+
you want to output in the speedscope json format (default), text, or a raw html
56+
viewer (currently broken).

‎README.md

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -59,19 +59,6 @@ Documentation HTML files can then be found in the `target/doc` directory.
5959

6060
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.
6161

62-
# Code organization
63-
64-
- `parser/src`: python lexing, parsing and ast
65-
- `vm/src`: python virtual machine
66-
- `builtins.rs`: Builtin functions
67-
- `compile.rs`: the python compiler from ast to bytecode
68-
- `obj`: python builtin types
69-
- `src`: using the other subcrates to bring rustpython to life.
70-
- `docs`: documentation (work in progress)
71-
- `py_code_object`: CPython bytecode to rustpython bytecode converter (work in progress)
72-
- `wasm`: Binary crate and resources for WebAssembly build
73-
- `tests`: integration test snippets
74-
7562
# Contributing
7663

7764
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
8875
`./whats_left.sh` to assist in finding any
8976
unimplemented method.
9077

91-
# Testing
92-
93-
To test rustpython, there is a collection of python snippets located in the
94-
`tests/snippets` directory. To run those tests do the following:
95-
96-
```shell
97-
$ cd tests
98-
$ pipenv install
99-
$ pipenv run pytest -v
100-
```
101-
102-
There also are some unit tests, you can run those with cargo:
103-
104-
```shell
105-
$ cargo test --all
106-
```
107-
108-
# Profiling
109-
110-
To profile rustpython, simply build in release mode with the `flame-it` feature.
111-
This will generate a file `flamescope.json`, which you can then view at
112-
https://speedscope.app.
113-
114-
```sh
115-
$ cargo run --release --features flame-it script.py
116-
$ cat flamescope.json
117-
{<json>}
118-
```
119-
120-
You can also pass the `--output-file` option to choose which file to output to
121-
(or stdout if you specify `-`), and the `--output-format` option to choose if
122-
you want to output in the speedscope json format (default), text, or a raw html
123-
viewer (currently broken).
124-
12578
# Using a standard library
12679

12780
As of now the standard library is under construction. You can
@@ -143,11 +96,6 @@ the [ouroboros library](https://github.com/pybee/ouroboros).
14396

14497
[See this doc](wasm/README.md)
14598

146-
# Code style
147-
148-
The code style used is the default [rustfmt](https://github.com/rust-lang/rustfmt) codestyle. Please format your code accordingly.
149-
We also use [clippy](https://github.com/rust-lang/rust-clippy) to detect rust code issues.
150-
15199
# Community
152100

153101
Chat with us on [gitter][gitter].

‎src/main.rs

Lines changed: 42 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,6 @@ use std::path::PathBuf;
2222
use std::process;
2323

2424
fn main() {
25-
if let Err(err) = run() {
26-
error!("Error: {}", err);
27-
process::exit(1);
28-
}
29-
}
30-
31-
fn run() -> Result<(), Box<dyn std::error::Error>> {
3225
#[cfg(feature = "flame-it")]
3326
let main_guard = flame::start_guard("RustPython main");
3427
env_logger::init();
@@ -62,19 +55,13 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
6255
Arg::with_name("profile_output")
6356
.long("profile-output")
6457
.takes_value(true)
65-
.help(
66-
"the file to output the profiling information to. present due to being \
67-
built with feature 'flame-it'",
68-
),
58+
.help("the file to output the profiling information to"),
6959
)
7060
.arg(
7161
Arg::with_name("profile_format")
7262
.long("profile-format")
7363
.takes_value(true)
74-
.help(
75-
"the profile format to output the profiling information in. present due to \
76-
being built with feature 'flame-it'",
77-
),
64+
.help("the profile format to output the profiling information in"),
7865
);
7966
let matches = app.get_matches();
8067

@@ -102,46 +89,53 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
10289

10390
#[cfg(feature = "flame-it")]
10491
{
105-
use std::fs::File;
106-
10792
main_guard.end();
108-
109-
enum ProfileFormat {
110-
Html,
111-
Text,
112-
Speedscope,
93+
if let Err(e) = write_profile(matches) {
94+
error!("Error writing profile information: {}", e);
95+
process::exit(1);
11396
}
97+
}
98+
}
11499

115-
let profile_output = matches.value_of_os("profile_output");
116-
117-
let profile_format = match matches.value_of("profile_format") {
118-
Some("html") => ProfileFormat::Html,
119-
Some("text") => ProfileFormat::Text,
120-
None if profile_output == Some("-".as_ref()) => ProfileFormat::Text,
121-
Some("speedscope") | None => ProfileFormat::Speedscope,
122-
Some(other) => {
123-
error!("Unknown profile format {}", other);
124-
process::exit(1);
125-
}
126-
};
100+
#[cfg(feature = "flame-it")]
101+
fn write_profile(matches: clap::ArgMatches) -> Result<(), Box<dyn std::error::Error>> {
102+
use std::fs::File;
127103

128-
let profile_output = profile_output.unwrap_or_else(|| match profile_format {
129-
ProfileFormat::Html => "flame-graph.html".as_ref(),
130-
ProfileFormat::Text => "flame.txt".as_ref(),
131-
ProfileFormat::Speedscope => "flamescope.json".as_ref(),
132-
});
104+
enum ProfileFormat {
105+
Html,
106+
Text,
107+
Speedscope,
108+
}
133109

134-
let profile_output: Box<dyn std::io::Write> = if profile_output == "-" {
135-
Box::new(std::io::stdout())
136-
} else {
137-
Box::new(File::create(profile_output)?)
138-
};
110+
let profile_output = matches.value_of_os("profile_output");
139111

140-
match profile_format {
141-
ProfileFormat::Html => flame::dump_html(profile_output)?,
142-
ProfileFormat::Text => flame::dump_text_to_writer(profile_output)?,
143-
ProfileFormat::Speedscope => flamescope::dump(profile_output)?,
112+
let profile_format = match matches.value_of("profile_format") {
113+
Some("html") => ProfileFormat::Html,
114+
Some("text") => ProfileFormat::Text,
115+
None if profile_output == Some("-".as_ref()) => ProfileFormat::Text,
116+
Some("speedscope") | None => ProfileFormat::Speedscope,
117+
Some(other) => {
118+
error!("Unknown profile format {}", other);
119+
process::exit(1);
144120
}
121+
};
122+
123+
let profile_output = profile_output.unwrap_or_else(|| match profile_format {
124+
ProfileFormat::Html => "flame-graph.html".as_ref(),
125+
ProfileFormat::Text => "flame.txt".as_ref(),
126+
ProfileFormat::Speedscope => "flamescope.json".as_ref(),
127+
});
128+
129+
let profile_output: Box<dyn std::io::Write> = if profile_output == "-" {
130+
Box::new(std::io::stdout())
131+
} else {
132+
Box::new(File::create(profile_output)?)
133+
};
134+
135+
match profile_format {
136+
ProfileFormat::Html => flame::dump_html(profile_output)?,
137+
ProfileFormat::Text => flame::dump_text_to_writer(profile_output)?,
138+
ProfileFormat::Speedscope => flamescope::dump(profile_output)?,
145139
}
146140

147141
Ok(())

0 commit comments

Comments
 (0)