servo: Merge #18351 - Refactor `.sort_by()` calls to use `.partial_cmp()` (from mateon1:profile-sort_by-partial_cmp); r=emilio
authorMateusz Naściszewski <matin1111@wp.pl>
Sat, 02 Sep 2017 03:54:49 -0500
changeset 427941 1f29a8f5c69dd6b0937c3368b7d088454b8e65d1
parent 427940 4fb65ac5a58719f8afe0fe5e4056f7e8f40ad846
child 427942 38a6312f4f8c5aebb4b6ca1d205e1992b434b780
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #18351 - Refactor `.sort_by()` calls to use `.partial_cmp()` (from mateon1:profile-sort_by-partial_cmp); r=emilio Changes the closures passed to `sort_by` in this file with a simpler, and more correct version. Previously, potential NaNs in the array would float to the top. Either way, the program would crash, as the `get_statistics` function asserts the array it gets is sorted, which always fails with a NaN. Because of that, this change should not affect functionality. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [ ] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [X] These changes do not require tests because statistics collected by --profile should not have NaN values in the first place. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 7dcd3ae50d090170fec4217248cc7864a1d2413a
servo/components/profile/time.rs
--- a/servo/components/profile/time.rs
+++ b/servo/components/profile/time.rs
@@ -350,23 +350,17 @@ impl Profiler {
                     Err(e) => panic!("Couldn't create {}: {}",
                                      path.display(),
                                      Error::description(&e)),
                     Ok(file) => file,
                 };
                 write!(file, "_category_\t_incremental?_\t_iframe?_\t_url_\t_mean (ms)_\t\
                     _median (ms)_\t_min (ms)_\t_max (ms)_\t_events_\n").unwrap();
                 for (&(ref category, ref meta), ref mut data) in &mut self.buckets {
-                    data.sort_by(|a, b| {
-                        if a < b {
-                            Ordering::Less
-                        } else {
-                            Ordering::Greater
-                        }
-                    });
+                    data.sort_by(|a, b| a.partial_cmp(b).expect("No NaN values in profiles"));
                     let data_len = data.len();
                     if data_len > 0 {
                         let (mean, median, min, max) = Self::get_statistics(data);
                         write!(file, "{}\t{}\t{:15.4}\t{:15.4}\t{:15.4}\t{:15.4}\t{:15}\n",
                             category.format(&self.output), meta.format(&self.output),
                             mean, median, min, max, data_len).unwrap();
                     }
                 }
@@ -375,23 +369,17 @@ impl Profiler {
                 let stdout = io::stdout();
                 let mut lock = stdout.lock();
 
                 writeln!(&mut lock, "{:35} {:14} {:9} {:30} {:15} {:15} {:-15} {:-15} {:-15}",
                          "_category_", "_incremental?_", "_iframe?_",
                          "            _url_", "    _mean (ms)_", "  _median (ms)_",
                          "     _min (ms)_", "     _max (ms)_", "      _events_").unwrap();
                 for (&(ref category, ref meta), ref mut data) in &mut self.buckets {
-                    data.sort_by(|a, b| {
-                        if a < b {
-                            Ordering::Less
-                        } else {
-                            Ordering::Greater
-                        }
-                    });
+                    data.sort_by(|a, b| a.partial_cmp(b).expect("No NaN values in profiles"));
                     let data_len = data.len();
                     if data_len > 0 {
                         let (mean, median, min, max) = Self::get_statistics(data);
                         writeln!(&mut lock, "{:-35}{} {:15.4} {:15.4} {:15.4} {:15.4} {:15}",
                                  category.format(&self.output), meta.format(&self.output), mean, median, min, max,
                                  data_len).unwrap();
                     }
                 }
@@ -413,23 +401,17 @@ impl Profiler {
                     password: password,
                     database: database,
                 };
 
                 let hosts = vec![hostname.as_str()];
                 let client = create_client(credentials, hosts);
 
                 for (&(ref category, ref meta), ref mut data) in &mut self.buckets {
-                    data.sort_by(|a, b| {
-                        if a < b {
-                            Ordering::Less
-                        } else {
-                            Ordering::Greater
-                        }
-                    });
+                    data.sort_by(|a, b| a.partial_cmp(b).expect("No NaN values in profiles"));
                     let data_len = data.len();
                     if data_len > 0 {
                         let (mean, median, min, max) = Self::get_statistics(data);
                         let category = category.format(&self.output);
                         let mut measurement = Measurement::new(&category);
                         measurement.add_field("mean", Value::Float(mean));
                         measurement.add_field("median", Value::Float(median));
                         measurement.add_field("min", Value::Float(min));