Hello,
Attached is an updated version.
Pádraig Brady wrote, On 03/06/2013 08:24 PM:
> On 03/06/2013 11:50 PM, Assaf Gordon wrote:
>> Attached is a suggestion to implement reservoir-sampling in shuf:
>> When the expected output of lines is known, it will not load the entire file
>> into memory - allowing shuffling very large inputs.
>>
Regarding comments:
>> {"-debug", no_argument, NULL, DEV_DEBUG_OPTION},
> no need to keep this, for final commit.
Yes, I'll remove this once the code is acceptable.
>> prepare_shuf_lines (struct linebuffer *in_lines, size_t n, char ***out_lines,
>
> I've not looked into the details, but it would
> be nice to avoid the memcpy/conversion here
>
I've removed the conversion function, and instead added a new function to
output the lines directly.
>> static size_t
>> read_input_reservoir_sampling (FILE *in, char eolbyte, char ***pline, size_t
>> k,
>> struct randint_source *s)
<...>
>> struct linebuffer *rsrv = XCALLOC (k, struct linebuffer); /* init
>> reservoir*/
>
> Since this change is mainly about efficient mem usage we should probably
> handle
> the case where we have small inputs but large k. This will allocate (and
> zero)
> memory up front. The zeroing will defeat any memory overcommit configured on
> the
> system, but it's probably better to avoid the large initial commit and realloc
> as required (not per line, but per 1K lines maybe).
>
I'm not quite sure about this:
The "reservoir-sampling" path can only be used when the user explicitly ask to
limit output lines.
I would naively assume that if a user explicitly asked to limit the output to
1,000,000 lines, he/she expects large input as well.
And so the (edge?) case of asking for a large number of output lines, but
supplying very small number of input lines is rare.
Wouldn't you agree? or is there a different typical usage case?
Also, the allocation only allocates an array of "struct linebuffer" (on 64bit
systems, 24 bytes).
So even asking for 1M lines will allocate 24MB of RAM - not "too much" on
modern machines.
----
The second attached patch is experimental - it tries to assess the randomness
of 'shuf' output by running it 1,000 times and checking if the output is (very
roughly) uniformly distributed.
I don't know if there were attempts in the past to unit-test randomness (and
then decided not to do it) - or if this was just never considered worth-while
(or too error prone).
Comments are welcomed,
-gordon
>From 1adfd08cd3a52c373932b0f1039755a240d2c0b8 Mon Sep 17 00:00:00 2001
From: Assaf Gordon <[email protected]>
Date: Thu, 7 Mar 2013 01:57:57 -0500
Subject: [PATCH 1/2] shuf: add (expensive) test for randomness
To run manually:
make check TESTS=tests/misc/shuf-nonrandomess.sh \
SUBDIRS=. RUN_VERY_EXPENSIVE_TESTS=yes
* tests/misc/shuf-randomness.sh: run 'shuf' repeatedly, and check if the
output is uniformly distributed "enough".
* tests/local.mk: add new test script.
---
tests/local.mk | 1 +
tests/misc/shuf-randomness.sh | 186 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 187 insertions(+), 0 deletions(-)
create mode 100755 tests/misc/shuf-randomness.sh
diff --git a/tests/local.mk b/tests/local.mk
index 607ddc4..d3923f8 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -313,6 +313,7 @@ all_tests = \
tests/misc/shred-passes.sh \
tests/misc/shred-remove.sh \
tests/misc/shuf.sh \
+ tests/misc/shuf-randomness.sh \
tests/misc/sort.pl \
tests/misc/sort-benchmark-random.sh \
tests/misc/sort-compress.sh \
diff --git a/tests/misc/shuf-randomness.sh b/tests/misc/shuf-randomness.sh
new file mode 100755
index 0000000..3e35cca
--- /dev/null
+++ b/tests/misc/shuf-randomness.sh
@@ -0,0 +1,186 @@
+#!/bin/sh
+# Test shuf for somewhat uniform randomness
+
+# Copyright (C) 2013 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ shuf
+getlimits_
+
+# Don't run these tests by default.
+very_expensive_
+
+# Number of trails
+T=1000
+
+# Number of categories
+N=100
+REQUIRED_CHI_SQUARED=200 # Be extremely leniet:
+ # don't require "great" goodness of fit
+ # even for our assumed 99 degrees of freedom
+
+# K - when testing reservoir-sampling, print K lines
+K=20
+REQUIRED_CHI_SQUARED_K=50 # Be extremely leniet:
+ # don't require "great" goodness of fit
+ # even for our assumed 19 degrees of freedom
+
+
+
+# The input: many zeros followed by 1 one
+(yes 0 | head -n $((N-1)) ; echo 1 ) > in || framework_failure_
+
+
+is_uniform()
+{
+ # Input is assumed to be a string of $T spaces-separated-values
+ # between 1 and $N
+ LINES="$1"
+
+ # Convert spaces to new-lines
+ LINES=$(echo "$LINES" | tr ' ' '\n' | sed '/^$/d') || framework_failure_
+
+ # Requre exactly $T values
+ COUNT=$(echo "$LINES" | wc -l)
+ test "$COUNT" -eq "$T" || framework_failure_
+
+ # HIST is the histogram of counts per categories
+ # ( categories are between 1 and $N )
+ HIST=$(echo "$LINES" | sort -n | uniq -c)
+
+ #DEBUG
+ #echo "HIST=$HIST" 1>&2
+
+ ## Calculate Chi-Squared
+ CHI=$( echo "$HIST" |
+ awk -v n=$N -v t=$T '{ counts[$2] = $1 }
+ END {
+ exptd = ((1.0)*t)/n
+ chi = 0
+ for (i=1;i<=n;++i)
+ {
+ if (i in counts)
+ obs = counts[i]
+ else
+ obs= 0;
+ chi += ((obs-exptd)*(obs-exptd))/exptd
+ }
+ printf "%7.0f\n", chi
+ }' ) || framework_failure_
+ #DEBUG
+ #echo "CHI = $CHI" 1>&2
+
+ # If CHI is "small enough" assume uniformity.
+ test "$CHI" -le "$REQUIRED_CHI_SQUARED" && return 0
+
+ # assume not uniform "enough"
+ return 1
+}
+
+is_uniform_k()
+{
+ K="$1"
+ LINES="$2"
+
+ # Convert spaces to new-lines
+ LINES=$(echo "$LINES" | tr ' ' '\n' | sed '/^$/d') || framework_failure_
+
+ # HIST is the histogram of counts per categories
+ # There should be K numeric categories (e.g 1/2/3/4/5 for K=5)
+ # and one X category (when the first K results from 'shuf' didn't
+ # contain "1").
+ HIST=$(echo "$LINES" | sort | uniq -c) || framework_failure_
+
+
+ #DEBUG
+ #echo "HIST=$HIST" 1>&2
+
+ ## Calculate Chi-Squared
+ CHI=$( echo "$HIST" |
+ awk -v n=$N \
+ -v t=$T \
+ -v k=$K \
+ '{ counts[$2] = $1 }
+ END {
+ # each of the first K categories is expected
+ # to appear as if they are uniformaly distributed
+ exptd_k = ((1.0)*t)/n
+
+ chi = 0
+ # Sum chi-squared for the K categories
+ for (i=1;i<=k;++i)
+ {
+ if (i in counts)
+ obs = counts[i]
+ else
+ obs= 0;
+ chi += ((obs-exptd_k)*(obs-exptd_k))/exptd_k
+ }
+
+ # Print Chi-Squared
+ printf "%7.0f\n",chi
+ }' ) || framework_failure_
+ #DEBUG
+ #echo "CHI = $CHI" 1>&2
+
+ # If CHI is "small enough" assume uniformity.
+ test "$CHI" -le "$REQUIRED_CHI_SQUARED_K" && return 0
+
+ # assume not uniform "enough"
+ return 1
+}
+
+
+##
+## Step 1:
+## Run a control: the "1"s in the input file should not be uniformly distributed
+##
+LINE=$(cat in | sed -n '/1/=') || framework_failure_
+for loop in $(seq $T) ;
+do
+ LINES="$LINES $LINE"
+done
+is_uniform "$LINES" && framework_failure_
+
+##
+## Step 2:
+## Shuffle the input files $T times.
+## The resulting position (=line number) of the "1" should be uniformly distributed.
+## This tests the "load entire file" control flow in the C code.
+LINES=""
+for loop in $(seq $T) ;
+do
+ LINE=$(shuf in | sed -n '/1/=') || framework_failure_
+ LINES="$LINES $LINE"
+done
+is_uniform "$LINES" ||
+ { fail=1 ; echo "step 2 failed - not uniformly distributed?" 1>&2 ; }
+
+##
+## Step 3:
+## Shuffle the input files $T times, selecting the first K lines.
+## This tests the "reservoir sampling" control flow in the C code.
+LINES=""
+for loop in $(seq $T) ;
+do
+ LINE=$(shuf -n $K in | sed -n '/1/=') || framework_failure_
+ LINES="$LINES $LINE"
+done
+is_uniform_k "$K" "$LINES" ||
+ { fail=1 ; echo "step 3 failed - not uniformly distributed?" 1>&2 ; }
+
+
+Exit $fail
--
1.7.7.4
>From d6d6e11fe399495fc1e7834ebd3e0cec94a7e044 Mon Sep 17 00:00:00 2001
From: Assaf Gordon <[email protected]>
Date: Wed, 6 Mar 2013 18:25:49 -0500
Subject: [PATCH 2/2] shuf: use reservoir-sampling when possible
Reservoir Sampling enables selecting K random lines from a very large
(or unknown-sized) input:
http://en.wikipedia.org/wiki/Reservoir_sampling
* src/shuf.c: Use reservoir-sampling when the number of output lines
is known (by using '-n X' parameter).
read_input_reservoir_sampling() - read lines from input file, and keep
only K lines in memory, replacing lines with decreasing probability.
write_permuted_output_reservoir() - output permuted reservoir lines.
main() - if the number of lines is known, use reservoir-sampling
instead of reading entire input file.
---
src/shuf.c | 144 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 138 insertions(+), 6 deletions(-)
diff --git a/src/shuf.c b/src/shuf.c
index 71ac3e6..7c5d16c 100644
--- a/src/shuf.c
+++ b/src/shuf.c
@@ -25,6 +25,7 @@
#include "error.h"
#include "fadvise.h"
#include "getopt.h"
+#include "linebuffer.h"
#include "quote.h"
#include "quotearg.h"
#include "randint.h"
@@ -81,7 +82,8 @@ With no FILE, or when FILE is -, read standard input.\n\
non-character as a pseudo short option, starting with CHAR_MAX + 1. */
enum
{
- RANDOM_SOURCE_OPTION = CHAR_MAX + 1
+ RANDOM_SOURCE_OPTION = CHAR_MAX + 1,
+ DEV_DEBUG_OPTION
};
static struct option const long_opts[] =
@@ -92,11 +94,31 @@ static struct option const long_opts[] =
{"output", required_argument, NULL, 'o'},
{"random-source", required_argument, NULL, RANDOM_SOURCE_OPTION},
{"zero-terminated", no_argument, NULL, 'z'},
+ {"-debug", no_argument, NULL, DEV_DEBUG_OPTION},
{GETOPT_HELP_OPTION_DECL},
{GETOPT_VERSION_OPTION_DECL},
{0, 0, 0, 0},
};
+/* debugging for developers. Enables devmsg(). */
+static bool dev_debug = false;
+
+/* Like error(0, 0, ...), but without an implicit newline.
+ Also a noop unless the global DEV_DEBUG is set.
+ TODO: Replace with variadic macro in system.h or
+ move to a separate module. */
+static inline void
+devmsg (char const *fmt, ...)
+{
+ if (dev_debug)
+ {
+ va_list ap;
+ va_start (ap, fmt);
+ vfprintf (stderr, fmt, ap);
+ va_end (ap);
+ }
+}
+
static bool
input_numbers_option_used (size_t lo_input, size_t hi_input)
{
@@ -135,6 +157,87 @@ next_line (char *line, char eolbyte, size_t n)
return p + 1;
}
+static size_t
+read_input_reservoir_sampling (FILE *in, char eolbyte, size_t k,
+ struct randint_source *s,
+ struct linebuffer **out_rsrv)
+{
+ size_t n_lines=0;
+ struct linebuffer line;
+ struct linebuffer *rsrv = XCALLOC (k, struct linebuffer); /* init reservoir*/
+
+ devmsg ("--reservoir_sampling--\n");
+
+ initbuffer (&line);
+ while (readlinebuffer_delim (&line, in, eolbyte)!=NULL)
+ {
+ if ( n_lines < k )
+ {
+ /* Read first K lines into reservoir */
+
+ if (dev_debug)
+ {
+ fprintf (stderr,"filling reservoir, input line %zu of %zu: '",
+ n_lines+1, k);
+ fwrite (line.buffer, sizeof (char), line.length-1, stderr);
+ fprintf (stderr, "'\n");
+ }
+
+ rsrv[n_lines] = line;
+ initbuffer (&line); /* next line-read will allocate a new buffer */
+
+
+ }
+ else
+ {
+ /* Read the rest of the lines, with decreasing probability of updating
+ the reservoir */
+ randint j = randint_choose (s, n_lines+1);
+ if ( j < k )
+ {
+ if (dev_debug)
+ {
+ fprintf (stderr,"Replacing reservoir sample %zu with " \
+ "line %zu '", j, n_lines);
+ fwrite (line.buffer, sizeof (char), line.length-1, stderr);
+ fprintf (stderr, "'\n");
+ }
+
+ rsrv[j] = line;
+ initbuffer (&line);/* next line-read will allocate a new buffer */
+
+ }
+ }
+
+ ++n_lines;
+ }
+ freebuffer (&line);
+
+ /* no more input lines, or an input error */
+ if (ferror (in))
+ error (EXIT_FAILURE, errno, _("read error"));
+
+ *out_rsrv = rsrv;
+ return MIN (k, n_lines);
+}
+
+static int
+write_permuted_output_reservoir (size_t n_lines, struct linebuffer *lines,
+ size_t const *permutation)
+{
+ size_t i;
+
+ for (i = 0; i < n_lines; i++)
+ {
+ const struct linebuffer *p = &lines[permutation[i]];
+ if (fwrite (p->buffer, sizeof (char),
+ p->length, stdout) != p->length)
+ return -1;
+ }
+
+ return 0;
+}
+
/* Read data from file IN. Input lines are delimited by EOLBYTE;
silently append a trailing EOLBYTE if the file ends in some other
byte. Store a pointer to the resulting array of lines into *PLINE.
@@ -209,14 +312,17 @@ main (int argc, char **argv)
char *random_source = NULL;
char eolbyte = '\n';
char **input_lines = NULL;
+ bool use_reservoir_sampling = false;
int optc;
int n_operands;
char **operand;
size_t n_lines;
- char **line;
+ char **line = NULL;
+ struct linebuffer *reservoir = NULL;
struct randint_source *randint_source;
size_t *permutation;
+ int i;
initialize_main (&argc, &argv);
set_program_name (argv[0]);
@@ -295,6 +401,10 @@ main (int argc, char **argv)
eolbyte = '\0';
break;
+ case DEV_DEBUG_OPTION:
+ dev_debug = true;
+ break;
+
case_GETOPT_HELP_CHAR;
case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS);
default:
@@ -341,8 +451,16 @@ main (int argc, char **argv)
fadvise (stdin, FADVISE_SEQUENTIAL);
- n_lines = read_input (stdin, eolbyte, &input_lines);
- line = input_lines;
+ if (head_lines != SIZE_MAX)
+ {
+ use_reservoir_sampling = true;
+ n_lines = SIZE_MAX; /* unknown number of input lines, for now */
+ }
+ else
+ {
+ n_lines = read_input (stdin, eolbyte, &input_lines);
+ line = input_lines;
+ }
}
head_lines = MIN (head_lines, n_lines);
@@ -352,6 +470,15 @@ main (int argc, char **argv)
if (! randint_source)
error (EXIT_FAILURE, errno, "%s", quotearg_colon (random_source));
+ if (use_reservoir_sampling)
+ {
+ /* Instead of reading the entire file into 'line',
+ use reservoir-sampling to store just "head_lines" random lines. */
+ n_lines = read_input_reservoir_sampling (stdin, eolbyte,
+ head_lines, randint_source,
+ &reservoir);
+ }
+
/* Close stdin now, rather than earlier, so that randint_all_new
doesn't have to worry about opening something other than
stdin. */
@@ -363,8 +490,13 @@ main (int argc, char **argv)
if (outfile && ! freopen (outfile, "w", stdout))
error (EXIT_FAILURE, errno, "%s", quotearg_colon (outfile));
- if (write_permuted_output (head_lines, line, lo_input, permutation, eolbyte)
- != 0)
+
+ if (use_reservoir_sampling)
+ i = write_permuted_output_reservoir (n_lines, reservoir, permutation);
+ else
+ i = write_permuted_output (head_lines, line, lo_input,
+ permutation, eolbyte);
+ if (i != 0)
error (EXIT_FAILURE, errno, _("write error"));
#ifdef lint
--
1.7.7.4