From: Luis Chamberlain <mcgrof@kernel.org>
To: mgorman@suse.de, jack@suse.cz, vbabka@suse.cz, dave@stgolabs.net,
ziy@nvidia.com, yuzhao@google.com, jhubbard@nvidia.com,
ryan.roberts@arm.com
Cc: kdevops@lists.linux.dev, p.raghav@samsung.com,
da.gomez@samsung.com, gost.dev@samsung.com,
Luis Chamberlain <mcgrof@kernel.org>
Subject: [mmtests RFT v2 2/2] MMTests::Stat: replace List::BinarySearch
Date: Thu, 23 May 2024 15:55:55 -0700 [thread overview]
Message-ID: <20240523225555.1791779-3-mcgrof@kernel.org> (raw)
In-Reply-To: <20240523225555.1791779-1-mcgrof@kernel.org>
List::BinarySearch is ancient and has a few issues:
a) The project hasnt't received any updates since 2018
b) It's licensed under GPLv1 or Artistica v1 license which are
not compatible with GPLv2
c) Some distributions don't carry a package for it
Debian in particular doesn't have it anymore. I looked at the code
and was bewildered with the amount of insane hacks to support ancient
versions of perl and to also support running our custom comparison
routine.
All the above challenges seem silly to deal with on users if we can
just implement what we need and call it a day. So let's do that.
Instead of piling a set of unit tests on mmtests the approach taken
to ensure a proper replacement for the library is to use a separate
repository that proves a replacement for the library, which has its
own unit tests [0]. The work we do here is then to not only show how
we have simplified the old code, but also can use the same tests in
the kill-List-BinarySearch against the exact old library without
breaking things. That gives us two ways to test for correctness.
While at it use SPDX license identifiers to simplify license clarity
just as we have in the kernel. The same SPDX tag is already upstream
on Linux. We can expand on this to keep things simpler later.
[0] https://github.com/mcgrof/kill-List-BinarySearch
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
README.md | 5 ++---
bin/install-depends | 1 -
bin/lib/MMTests/Stat.pm | 48 +++++++++++++++++++++++++++++++++++++++--
compare-kernels.sh | 5 -----
4 files changed, 48 insertions(+), 11 deletions(-)
diff --git a/README.md b/README.md
index ae9b119d150d..98c5975e4197 100644
--- a/README.md
+++ b/README.md
@@ -44,9 +44,8 @@ $ ../../compare-kernels.sh --format html --output-dir /tmp/html > /tmp/html/inde
The first step is optional. Some configurations are auto-generated from
a template, particularly the filesystem-specific ones.
-Note that [`List::BinarySearch`](https://metacpan.org/pod/List::BinarySearch)
-and maybe even [`Math::Gradient`](https://metacpan.org/pod/Math::Gradient) may
-need to be installed from [CPAN](https://www.cpan.org/) for the reporting to
+Note that perhaps [`Math::Gradient`](https://metacpan.org/pod/Math::Gradient)
+may need to be installed from [CPAN](https://www.cpan.org/) for the reporting to
work. Similarly, [`R`](https://www.r-project.org/) should be installed if
attempting to highlight whether performance differences are statistically
relevant.
diff --git a/bin/install-depends b/bin/install-depends
index 4539350868c3..860995488680 100755
--- a/bin/install-depends
+++ b/bin/install-depends
@@ -61,7 +61,6 @@ my %package_map = (
"debian::xz" => "xz-utils",
"debian::zeromq-devel" => "libzmq3-dev",
"debian::zlib-devel" => "zlib1g-dev",
- "debian::perl-List-BinarySearch" => "liblist-binarysearch-perl",
"debian::perl-GD" => "libgd-perl",
"debian::sysvinit-tools" => "sysvinit-utils",
"debian::gettext-runtime" => "gettext",
diff --git a/bin/lib/MMTests/Stat.pm b/bin/lib/MMTests/Stat.pm
index 41e6b79267c9..d14b51bb36c3 100644
--- a/bin/lib/MMTests/Stat.pm
+++ b/bin/lib/MMTests/Stat.pm
@@ -1,16 +1,23 @@
+# SPDX-License-Identifier: GPL-2.0-or-later OR copyleft-next-0.3.1
#
# Stat.pm
#
# Basic stats module
+#
+# Copyright (c) 2002-2024 Mel Gorman <mgorman@techsingularity.net>
+# Copyright (c) 2017-2019 Jan Kara <jack@suse.cz>
+# Copyright (c) 2017-2023 Andreas Herrmann <aherrmann@suse.de>
+# Copyright (c) 2024 Luis Chamberlain <mcgrof@kernel.org>
package MMTests::Stat;
require Exporter;
use vars qw (@ISA @EXPORT);
use MMTests::Report;
use strict;
+use feature 'signatures';
+no warnings 'experimental::signatures';
use POSIX qw(floor);
use FindBin qw($Bin);
-use List::BinarySearch qw(binsearch_range);
use Scalar::Util qw(looks_like_number);
@ISA = qw(Exporter);
@@ -479,6 +486,43 @@ sub calc_submean_ci {
return ($parsedrow[1], $parsedrow[2]);
}
+sub binsearch_pos_num($target, $aref) {
+ die "Only numbers allowed" unless looks_like_number($target);
+ die "Expected an array reference!" unless ref $aref eq 'ARRAY';
+
+ my ($low, $high) = (0, scalar @$aref - 1);
+
+ while ($low <= $high) {
+ my $mid = int(($low + $high) / 2);
+ if ($aref->[$mid] < $target) {
+ $low = $mid + 1;
+ } elsif ($aref->[$mid] > $target) {
+ $high = $mid - 1;
+ } else {
+ return $mid;
+ }
+ }
+
+ return $low;
+}
+
+# Returns an inclusive range for both, if you only use one return
+# value, you consume the last value.
+sub binsearch_range_num ($low_target, $high_target, $aref) {
+ die "Only numbers allowed" unless looks_like_number($low_target);
+ die "Only numbers allowed" unless looks_like_number($high_target);
+ die "Expected an array reference!" unless ref $aref eq 'ARRAY';
+
+ my $index_low = binsearch_pos_num($low_target, $aref);
+ my $index_high = binsearch_pos_num($high_target, $aref);
+
+ if($index_high == scalar @$aref or $aref->[$index_high] > $high_target) {
+ $index_high--;
+ }
+
+ return ($index_low, $index_high);
+}
+
sub calc_samples {
my ($dataref, $arg) = @_;
my $elements = @{$dataref};
@@ -495,7 +539,7 @@ sub calc_samples {
} elsif ($high eq "max") {
$high = $dataref->[$elements - 1];
}
- my ($lowidx, $highidx) = binsearch_range { $a <=> $b } $low, $high, @{$dataref};
+ my ($lowidx, $highidx) = binsearch_range_num($low, $high, $dataref);
return $highidx - $lowidx + 1;
}
diff --git a/compare-kernels.sh b/compare-kernels.sh
index 7c49f2d95044..428de060c553 100755
--- a/compare-kernels.sh
+++ b/compare-kernels.sh
@@ -83,11 +83,6 @@ if [ ! -t 1 ]; then
OUT=$(mktemp ~/.compare-kernel-XXXX.out)
fi
-perldoc -l List::BinarySearch &>/dev/null
-if [ $? -ne 0 ]; then
- install-depends perl-List-BinarySearch &> $OUT
-fi
-
if [ "$FORMAT" = "html" ]; then
install-depends gnuplot &>> $OUT
install-depends perl-GD &>> $OUT
--
2.43.0
next prev parent reply other threads:[~2024-05-23 22:56 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-23 22:55 [mmtests RFT v2 0/2] mmtests: fix debian dependencies Luis Chamberlain
2024-05-23 22:55 ` [mmtests RFT v2 1/2] bin/install-depends: call ldconfig Luis Chamberlain
2024-05-24 12:45 ` Jan Kara
2024-05-23 22:55 ` Luis Chamberlain [this message]
2024-05-24 12:52 ` [mmtests RFT v2 2/2] MMTests::Stat: replace List::BinarySearch Jan Kara
2024-05-24 15:55 ` [mmtests RFT v2 0/2] mmtests: fix debian dependencies Davidlohr Bueso
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240523225555.1791779-3-mcgrof@kernel.org \
--to=mcgrof@kernel.org \
--cc=da.gomez@samsung.com \
--cc=dave@stgolabs.net \
--cc=gost.dev@samsung.com \
--cc=jack@suse.cz \
--cc=jhubbard@nvidia.com \
--cc=kdevops@lists.linux.dev \
--cc=mgorman@suse.de \
--cc=p.raghav@samsung.com \
--cc=ryan.roberts@arm.com \
--cc=vbabka@suse.cz \
--cc=yuzhao@google.com \
--cc=ziy@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox