From db6fa17ffedb7f4439b3ffdba6330bb157a47e24 Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Mon, 5 Apr 2021 09:49:16 -0500 Subject: [PATCH 1/3] Don't recompile CPython microbenchmarks every time --- benches/microbenchmarks.rs | 44 ++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/benches/microbenchmarks.rs b/benches/microbenchmarks.rs index a344e3aec..d0f23f160 100644 --- a/benches/microbenchmarks.rs +++ b/benches/microbenchmarks.rs @@ -7,7 +7,7 @@ use rustpython_vm::pyobject::ItemProtocol; use rustpython_vm::pyobject::PyResult; use rustpython_vm::{InitParameter, Interpreter, PySettings}; use std::path::{Path, PathBuf}; -use std::{fs, io}; +use std::{ffi, fs, io}; pub struct MicroBenchmark { name: String, @@ -20,8 +20,16 @@ fn bench_cpython_code(group: &mut BenchmarkGroup, bench: &MicroBenchma let gil = cpython::Python::acquire_gil(); let py = gil.python(); - let bench_func = |(code, globals, locals)| { - let res = cpy_run_code(py, &code, &globals, &locals); + let setup_code = ffi::CString::new(&*bench.setup).unwrap(); + let setup_name = ffi::CString::new(format!("{}_setup", bench.name)).unwrap(); + let setup_code = cpy_compile_code(py, &setup_code, &setup_name).unwrap(); + + let code = ffi::CString::new(&*bench.code).unwrap(); + let name = ffi::CString::new(&*bench.name).unwrap(); + let code = cpy_compile_code(py, &code, &name).unwrap(); + + let bench_func = |(globals, locals): &mut (cpython::PyDict, cpython::PyDict)| { + let res = cpy_run_code(py, &code, globals, locals); if let Err(e) = res { e.print(py); panic!("Error running microbenchmark") @@ -30,36 +38,36 @@ fn bench_cpython_code(group: &mut BenchmarkGroup, bench: &MicroBenchma let bench_setup = |iterations| { let globals = cpython::PyDict::new(py); + // setup the __builtins__ attribute - no other way to do this (other than manually) as far + // as I can tell + let _ = py.run("", Some(&globals), None); let locals = cpython::PyDict::new(py); if let Some(idx) = iterations { globals.set_item(py, "ITERATIONS", idx).unwrap(); } - let res = py.run(&bench.setup, Some(&globals), Some(&locals)); + let res = cpy_run_code(py, &setup_code, &globals, &locals); if let Err(e) = res { e.print(py); panic!("Error running microbenchmark setup code") } - let code = std::ffi::CString::new(&*bench.code).unwrap(); - let name = std::ffi::CString::new(&*bench.name).unwrap(); - let code = cpy_compile_code(py, &code, &name).unwrap(); - (code, globals, locals) + (globals, locals) }; if bench.iterate { for idx in (100..=1_000).step_by(200) { group.throughput(Throughput::Elements(idx as u64)); group.bench_with_input(BenchmarkId::new("cpython", &bench.name), &idx, |b, idx| { - b.iter_batched( + b.iter_batched_ref( || bench_setup(Some(*idx)), bench_func, - BatchSize::PerIteration, + BatchSize::LargeInput, ); }); } } else { group.bench_function(BenchmarkId::new("cpython", &bench.name), move |b| { - b.iter_batched(|| bench_setup(None), bench_func, BatchSize::PerIteration); + b.iter_batched_ref(|| bench_setup(None), bench_func, BatchSize::LargeInput); }); } } @@ -73,8 +81,8 @@ unsafe fn cpy_res( fn cpy_compile_code( py: cpython::Python<'_>, - s: &std::ffi::CStr, - fname: &std::ffi::CStr, + s: &ffi::CStr, + fname: &ffi::CStr, ) -> cpython::PyResult { unsafe { let res = @@ -114,8 +122,8 @@ fn bench_rustpy_code(group: &mut BenchmarkGroup, bench: &MicroBenchmar .compile(&bench.code, Mode::Exec, bench.name.to_owned()) .expect("Error compiling bench code"); - let bench_func = |(scope, bench_code)| { - let res: PyResult = vm.run_code_obj(bench_code, scope); + let bench_func = |scope| { + let res: PyResult = vm.run_code_obj(bench_code.clone(), scope); vm.unwrap_pyresult(res); }; @@ -129,7 +137,7 @@ fn bench_rustpy_code(group: &mut BenchmarkGroup, bench: &MicroBenchmar } let setup_result = vm.run_code_obj(setup_code.clone(), scope.clone()); vm.unwrap_pyresult(setup_result); - (scope, bench_code.clone()) + scope }; if bench.iterate { @@ -142,14 +150,14 @@ fn bench_rustpy_code(group: &mut BenchmarkGroup, bench: &MicroBenchmar b.iter_batched( || bench_setup(Some(*idx)), bench_func, - BatchSize::PerIteration, + BatchSize::LargeInput, ); }, ); } } else { group.bench_function(BenchmarkId::new("rustpython", &bench.name), move |b| { - b.iter_batched(|| bench_setup(None), bench_func, BatchSize::PerIteration); + b.iter_batched(|| bench_setup(None), bench_func, BatchSize::LargeInput); }); } }) From e6b52193932da0d337ba4bc544920426641134fc Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Mon, 5 Apr 2021 09:50:15 -0500 Subject: [PATCH 2/3] Fix remaining test_random overflows --- Lib/test/test_random.py | 4 ---- vm/src/stdlib/random.rs | 16 +++++++++++----- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_random.py b/Lib/test/test_random.py index c9f463d09..0c7b64040 100644 --- a/Lib/test/test_random.py +++ b/Lib/test/test_random.py @@ -422,10 +422,6 @@ class MersenneTwister_TestBasicOps(TestBasicOps, unittest.TestCase): def test_seedargs(self): super().test_seedargs() - @unittest.skip("TODO: RUSTPYTHON, hangs?") - def test_choice(self): - super().test_choice() - # TODO: RUSTPYTHON @unittest.expectedFailure def test_pickling(self): diff --git a/vm/src/stdlib/random.rs b/vm/src/stdlib/random.rs index dfe63654e..cc8735c27 100644 --- a/vm/src/stdlib/random.rs +++ b/vm/src/stdlib/random.rs @@ -106,13 +106,19 @@ mod _random { } #[pymethod] - fn getrandbits(&self, k: usize) -> BigInt { + fn getrandbits(&self, k: usize, vm: &VirtualMachine) -> PyResult { + if k <= 0 { + return Err( + vm.new_value_error("number of bits must be greater than zero".to_owned()) + ); + } + let mut rng = self.rng.lock(); let mut k = k; - let mut gen_u32 = |k| rng.next_u32() >> 32usize.wrapping_sub(k) as u32; + let mut gen_u32 = |k| rng.next_u32().wrapping_shr(32usize.wrapping_sub(k) as u32); if k <= 32 { - return gen_u32(k).into(); + return Ok(gen_u32(k).into()); } let words = (k - 1) / 8 + 1; @@ -123,10 +129,10 @@ mod _random { let it = it.rev(); for word in it { *word = gen_u32(k); - k -= 32; + k = k.wrapping_sub(32); } - BigInt::from_slice(Sign::NoSign, &wordarray) + Ok(BigInt::from_slice(Sign::NoSign, &wordarray)) } } } From f9555fd75d7b6a47d69ba124242ed11fb5db758a Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Tue, 6 Apr 2021 11:46:33 -0500 Subject: [PATCH 3/3] Fix getrandbits for k>32 --- Cargo.lock | 4 ++-- Lib/test/test_random.py | 9 --------- vm/src/stdlib/random.rs | 40 ++++++++++++++++++++++++++-------------- 3 files changed, 28 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dbc0993d6..cda08d499 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1201,9 +1201,9 @@ dependencies = [ [[package]] name = "mt19937" -version = "2.0.0" +version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3e6dd1b4462edbfbc0c4ad4c3205d94623bb94b4819aa0888936988d38834158" +checksum = "12ca7f22ed370d5991a9caec16a83187e865bc8a532f889670337d5a5689e3a1" dependencies = [ "rand_core 0.6.2", ] diff --git a/Lib/test/test_random.py b/Lib/test/test_random.py index 0c7b64040..4648e2742 100644 --- a/Lib/test/test_random.py +++ b/Lib/test/test_random.py @@ -427,11 +427,6 @@ class MersenneTwister_TestBasicOps(TestBasicOps, unittest.TestCase): def test_pickling(self): super().test_pickling() - # TODO: RUSTPYTHON - @unittest.expectedFailure - def test_bug_9025(self): - super().test_bug_9025() - # TODO: RUSTPYTHON @unittest.expectedFailure def test_seed_when_randomness_source_not_found(self): @@ -609,8 +604,6 @@ class MersenneTwister_TestBasicOps(TestBasicOps, unittest.TestCase): cum |= int(self.gen.random() * span) self.assertEqual(cum, span-1) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_bigrand(self): # The randrange routine should build-up the required number of bits # in stages so that all bit positions are active. @@ -661,8 +654,6 @@ class MersenneTwister_TestBasicOps(TestBasicOps, unittest.TestCase): self.assertRaises(ValueError, self.gen.getrandbits, 0) self.assertRaises(ValueError, self.gen.getrandbits, -1) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_randrange_uses_getrandbits(self): # Verify use of getrandbits by randrange # Use same seed as in the cross-platform repeatability test diff --git a/vm/src/stdlib/random.rs b/vm/src/stdlib/random.rs index cc8735c27..9acc75ad7 100644 --- a/vm/src/stdlib/random.rs +++ b/vm/src/stdlib/random.rs @@ -11,7 +11,7 @@ mod _random { use crate::pyobject::{BorrowValue, PyObjectRef, PyRef, PyResult, PyValue, StaticType}; use crate::VirtualMachine; use num_bigint::{BigInt, Sign}; - use num_traits::Signed; + use num_traits::{Signed, Zero}; use rand::{rngs::StdRng, RngCore, SeedableRng}; #[derive(Debug)] @@ -107,7 +107,7 @@ mod _random { #[pymethod] fn getrandbits(&self, k: usize, vm: &VirtualMachine) -> PyResult { - if k <= 0 { + if k == 0 { return Err( vm.new_value_error("number of bits must be greater than zero".to_owned()) ); @@ -115,24 +115,36 @@ mod _random { let mut rng = self.rng.lock(); let mut k = k; - let mut gen_u32 = |k| rng.next_u32().wrapping_shr(32usize.wrapping_sub(k) as u32); + let mut gen_u32 = |k| { + let r = rng.next_u32(); + if k < 32 { + r >> (32 - k) + } else { + r + } + }; if k <= 32 { return Ok(gen_u32(k).into()); } - let words = (k - 1) / 8 + 1; - let mut wordarray = vec![0u32; words]; + let words = (k - 1) / 32 + 1; + let wordarray = (0..words) + .map(|_| { + let word = gen_u32(k); + k = k.wrapping_sub(32); + word + }) + .collect::>(); - let it = wordarray.iter_mut(); - #[cfg(target_endian = "big")] - let it = it.rev(); - for word in it { - *word = gen_u32(k); - k = k.wrapping_sub(32); - } - - Ok(BigInt::from_slice(Sign::NoSign, &wordarray)) + let uint = num_bigint::BigUint::new(wordarray); + // very unlikely but might as well check + let sign = if uint.is_zero() { + Sign::NoSign + } else { + Sign::Plus + }; + Ok(BigInt::from_biguint(sign, uint)) } } }