From 1ecb52e0e7fad29e8a185a6505faec7d667b5801 Mon Sep 17 00:00:00 2001 From: John Kerl Date: Mon, 14 Sep 2015 21:00:21 -0400 Subject: [PATCH 1/5] todo --- c/input/line_readers.c | 43 ++++++++++++++++++++++++++++++++++++++++++ c/todo.txt | 36 +++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 c/input/line_readers.c diff --git a/c/input/line_readers.c b/c/input/line_readers.c new file mode 100644 index 000000000..eb04a5881 --- /dev/null +++ b/c/input/line_readers.c @@ -0,0 +1,43 @@ +#include +#include "lib/mlrutil.h" + +// xxx under construction + +// Use powers of two exclusively, to help avoid heap fragmentation +#define INITIAL_SIZE 128 + +// xxx look up what restrict do ... should i be using these more often? +// xxx limited semantics: initial linep & linecapp are disregarded; doesn't return the delimiter in the string. +// xxx getcdelim is just for comparison to getdelim. getsdelim is the deliverable. +size_t mlr_getcdelim(char ** restrict ppline, size_t * restrict plinecap, int delimiter, FILE * restrict fp) { + size_t linecap = INITIAL_SIZE; + char* pline = mlr_malloc_or_die(INITIAL_SIZE); + char* p = pline; + int len = 0; + + while (TRUE) { + if (len >= linecap) { + linecap = linecap << 1; + // xxx mlr_realloc_or_die + pline = realloc(pline, linecap); + p = pline; + } + int c = getc_unlocked(fp); + if (c == EOF) { + break; + } else if (c == delimiter) { + *(p++) = 0; + } else { + *(p++) = c; + } + } + + *ppline = pline; + *plinecap = linecap; + len = p - pline; + return len; +} + +size_t mlr_getsdelim(char ** restrict ppline, size_t * restrict plinecap, char* delimiter, FILE * restrict fp) { + return 0; // xxx stub +} diff --git a/c/todo.txt b/c/todo.txt index ac33d8d43..bc9fec2b3 100644 --- a/c/todo.txt +++ b/c/todo.txt @@ -9,6 +9,41 @@ TOP OF LIST ---------------------------------------------------------------- MAJOR: autoconfig +---------------------------------------------------------------- +PLANNING NOTES + +multi-char IXS for other-than-CSV +* don't use parse-trie unless someone really wants double-quotes in DKVP (i don't need it, and no one + else has asked for it) +* requires either: + y multi-char getdelim + - just need to code it up and iterate on the perf work + n or use pfr/pbr + - performance improvements quite likely in the mmap/stringptr case (see CSV) + - performance for the stdio case will be helped by multi-char getdelim, but will still likely to be much slower + than getdelim. and double-buffering pbr on top of mcgetdelim is needless copying + - pfr/pbr probably too heavyweight to not cause a performance regression + - pfr/pbr aren't really important in the absence of parse-trie +* probably keep the single-char pfuncs as-is, to be invoked in single-char + cases -- unless perf measurements show there's not a significant loss + +CSV performance/memmgt issues: +* mmap/stringptr cases: + o parse-trie must be kept + o need to impl zero-strdup logic for sure -- this will be a significant gain + o replace pfr/pbr with zero-copy pointer arithmetic (the backing *is* the buffer) +* stdio: + o parse-trie must be kept + o multi-char getdelim will probably be needed; pfr/pbr is too slow even with ring-buffer & inlining + +in summary: pfr/pbr have got to go + +multi-char getdelim +* start with poor-man's version which calls single-char getdelim with irs[-1] & strcats in case !matchback + o --exit-stats option? for hash-stats too? +* separately, code up a genuine multi-char getdelim + o modify the poor-man's logic but simply keep appending in-routine in the !matchback case + ---------------------------------------------------------------- MAJOR: multi-char separators for file formats other than CSV k oxs is functionally done @@ -45,6 +80,7 @@ MAJOR: csv mem-leak/read-perf MINOR * define dkvp, nidx, etc @ cover x 2 + - web-server access logs in dkvp format -- ? ? dkvp quoting ... wait until after the mmap/perf split. else, very undesirable performance regression. From 8a8784c77fb57a024f450cb3318dfa0b4003dced Mon Sep 17 00:00:00 2001 From: John Kerl Date: Mon, 14 Sep 2015 22:12:46 -0400 Subject: [PATCH 2/5] iterate on read-performance experiments --- c/Makefile | 1 + c/experimental/getlines.c | 32 ++++++++++++++++++++++++++++++++ c/input/line_readers.c | 21 +++++++++++++++------ 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/c/Makefile b/c/Makefile index 49a744250..b695d07c8 100644 --- a/c/Makefile +++ b/c/Makefile @@ -102,6 +102,7 @@ lib/mlr_globals.c \ lib/string_builder.c \ input/stdio_byte_reader.c \ input/file_reader_mmap.c \ +input/line_readers.c \ containers/parse_trie.c \ experimental/getlines.c diff --git a/c/experimental/getlines.c b/c/experimental/getlines.c index c444613d0..03d020cd7 100644 --- a/c/experimental/getlines.c +++ b/c/experimental/getlines.c @@ -7,6 +7,7 @@ #include "input/lrec_readers.h" #include "lib/string_builder.h" #include "input/byte_readers.h" +#include "input/line_readers.h" #include "input/peek_file_reader.h" #include "containers/parse_trie.h" @@ -44,6 +45,29 @@ static int read_file_mlr_get_line(char* filename, int do_write) { return bc; } +// ================================================================ +static int read_file_mlr_getcdelim(char* filename, int do_write) { + FILE* fp = fopen_or_die(filename); + char irs = '\n'; + int bc = 0; + while (1) { + char* line = NULL; + size_t linecap = 0; + ssize_t linelen = mlr_getcdelim(&line, &linecap, irs, fp); + if (linelen < 0) { + break; + } + bc += linelen; + if (do_write) { + fputs(line, stdout); + fputc('\n', stdout); + } + free(line); + } + fclose(fp); + return bc; +} + // ================================================================ static char* read_line_fgetc(FILE* fp, char* irs, int irs_len) { char* line = mlr_malloc_or_die(FIXED_LINE_LEN); @@ -347,6 +371,7 @@ int main(int argc, char** argv) { int bc; for (int i = 0; i < nreps; i++) { + s = get_systime(); bc = read_file_mlr_get_line(filename, do_write); e = get_systime(); @@ -354,6 +379,13 @@ int main(int argc, char** argv) { printf("type=getdelim,t=%.6lf,n=%d\n", t, bc); fflush(stdout); + s = get_systime(); + bc = read_file_mlr_getcdelim(filename, do_write); + e = get_systime(); + t = e - s; + printf("type=mlr_getcdelim,t=%.6lf,n=%d\n", t, bc); + fflush(stdout); + s = get_systime(); bc = read_file_fgetc_fixed_len(filename, do_write); e = get_systime(); diff --git a/c/input/line_readers.c b/c/input/line_readers.c index eb04a5881..9841173b6 100644 --- a/c/input/line_readers.c +++ b/c/input/line_readers.c @@ -13,10 +13,10 @@ size_t mlr_getcdelim(char ** restrict ppline, size_t * restrict plinecap, int de size_t linecap = INITIAL_SIZE; char* pline = mlr_malloc_or_die(INITIAL_SIZE); char* p = pline; - int len = 0; + int eof = FALSE; while (TRUE) { - if (len >= linecap) { + if ((p-pline) >= linecap) { linecap = linecap << 1; // xxx mlr_realloc_or_die pline = realloc(pline, linecap); @@ -24,18 +24,27 @@ size_t mlr_getcdelim(char ** restrict ppline, size_t * restrict plinecap, int de } int c = getc_unlocked(fp); if (c == EOF) { + if (p == pline) + eof = TRUE; + *(p++) = 0; break; } else if (c == delimiter) { *(p++) = 0; + break; } else { *(p++) = c; } } - *ppline = pline; - *plinecap = linecap; - len = p - pline; - return len; + if (eof) { + free(pline); + *ppline = NULL; + return -1; + } else { + *ppline = pline; + *plinecap = linecap; + return p - pline; + } } size_t mlr_getsdelim(char ** restrict ppline, size_t * restrict plinecap, char* delimiter, FILE * restrict fp) { From 736c66ae1071c146de0e97aaa88bdf742c4e5a9f Mon Sep 17 00:00:00 2001 From: John Kerl Date: Mon, 14 Sep 2015 22:36:02 -0400 Subject: [PATCH 3/5] iterate on read-performance experiments --- c/experimental/getlines.c | 31 ++++++++++++++++++ c/input/line_readers.c | 69 ++++++++++++++++++++++++++++++++------- 2 files changed, 88 insertions(+), 12 deletions(-) diff --git a/c/experimental/getlines.c b/c/experimental/getlines.c index 03d020cd7..1af42fde3 100644 --- a/c/experimental/getlines.c +++ b/c/experimental/getlines.c @@ -68,6 +68,30 @@ static int read_file_mlr_getcdelim(char* filename, int do_write) { return bc; } +// ================================================================ +static int read_file_mlr_getsdelim(char* filename, int do_write) { + FILE* fp = fopen_or_die(filename); + char* irs = "\r\n"; + int irslen = strlen(irs); + int bc = 0; + while (1) { + char* line = NULL; + size_t linecap = 0; + ssize_t linelen = mlr_getsdelim(&line, &linecap, irs, irslen, fp); + if (linelen < 0) { + break; + } + bc += linelen; + if (do_write) { + fputs(line, stdout); + fputc('\n', stdout); + } + free(line); + } + fclose(fp); + return bc; +} + // ================================================================ static char* read_line_fgetc(FILE* fp, char* irs, int irs_len) { char* line = mlr_malloc_or_die(FIXED_LINE_LEN); @@ -386,6 +410,13 @@ int main(int argc, char** argv) { printf("type=mlr_getcdelim,t=%.6lf,n=%d\n", t, bc); fflush(stdout); + s = get_systime(); + bc = read_file_mlr_getsdelim(filename, do_write); + e = get_systime(); + t = e - s; + printf("type=mlr_getsdelim,t=%.6lf,n=%d\n", t, bc); + fflush(stdout); + s = get_systime(); bc = read_file_fgetc_fixed_len(filename, do_write); e = get_systime(); diff --git a/c/input/line_readers.c b/c/input/line_readers.c index 9841173b6..8c4322dc1 100644 --- a/c/input/line_readers.c +++ b/c/input/line_readers.c @@ -11,25 +11,25 @@ // xxx getcdelim is just for comparison to getdelim. getsdelim is the deliverable. size_t mlr_getcdelim(char ** restrict ppline, size_t * restrict plinecap, int delimiter, FILE * restrict fp) { size_t linecap = INITIAL_SIZE; - char* pline = mlr_malloc_or_die(INITIAL_SIZE); - char* p = pline; + char* restrict pline = mlr_malloc_or_die(INITIAL_SIZE); + char* restrict p = pline; int eof = FALSE; + int c; while (TRUE) { if ((p-pline) >= linecap) { linecap = linecap << 1; - // xxx mlr_realloc_or_die - pline = realloc(pline, linecap); + pline = realloc(pline, linecap); // xxx mlr_realloc_or_die p = pline; } - int c = getc_unlocked(fp); - if (c == EOF) { + c = getc_unlocked(fp); + if (c == delimiter) { + *p = 0; + break; + } else if (c == EOF) { if (p == pline) eof = TRUE; - *(p++) = 0; - break; - } else if (c == delimiter) { - *(p++) = 0; + *p = 0; break; } else { *(p++) = c; @@ -47,6 +47,51 @@ size_t mlr_getcdelim(char ** restrict ppline, size_t * restrict plinecap, int de } } -size_t mlr_getsdelim(char ** restrict ppline, size_t * restrict plinecap, char* delimiter, FILE * restrict fp) { - return 0; // xxx stub +size_t mlr_getsdelim(char ** restrict ppline, size_t * restrict plinecap, char* delimiter, int delimlen, + FILE * restrict fp) +{ + size_t linecap = INITIAL_SIZE; + char* restrict pline = mlr_malloc_or_die(INITIAL_SIZE); + char* restrict p = pline; + int eof = FALSE; + int c; + int delimlen1 = delimlen - 1; + int delimlast = delimiter[delimlen1]; + + while (TRUE) { + if ((p-pline) >= linecap) { + linecap = linecap << 1; + pline = realloc(pline, linecap); // xxx mlr_realloc_or_die + p = pline; + } + c = getc_unlocked(fp); + if (c == delimlast) { + // Example: delim="abc". last='c'. Already have read "ab" into pline. p-pline=2. + // Now reading 'c'. + // xxx make a memeq + if (((p-pline) >= delimlen1) && !strncmp(p-delimlen1, delimiter, delimlen1)) { + *p = 0; + break; + } else { + *(p++) = c; + } + } else if (c == EOF) { + if (p == pline) + eof = TRUE; + *p = 0; + break; + } else { + *(p++) = c; + } + } + + if (eof) { + free(pline); + *ppline = NULL; + return -1; + } else { + *ppline = pline; + *plinecap = linecap; + return p - pline; + } } From 5b78102fc292e45f84169e5bf607f88970af75ce Mon Sep 17 00:00:00 2001 From: John Kerl Date: Mon, 14 Sep 2015 22:37:07 -0400 Subject: [PATCH 4/5] iterate on read-performance experiments --- c/input/line_readers.h | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 c/input/line_readers.h diff --git a/c/input/line_readers.h b/c/input/line_readers.h new file mode 100644 index 000000000..3da241921 --- /dev/null +++ b/c/input/line_readers.h @@ -0,0 +1,9 @@ +#ifndef LINE_READERS_H +#define LINE_READERS_H + +#include + +size_t mlr_getcdelim(char ** restrict ppline, size_t * restrict plinecap, int delimiter, FILE * restrict fp); +size_t mlr_getsdelim(char ** restrict ppline, size_t * restrict plinecap, char* delimiter, int delimlen, FILE * restrict fp); + +#endif // LINE_READERS_H From c678cb41c12bfcb4889c9136212f97a638df4bea Mon Sep 17 00:00:00 2001 From: John Kerl Date: Mon, 14 Sep 2015 22:42:59 -0400 Subject: [PATCH 5/5] iterate on read-performance experiments --- c/experimental/getlines.c | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/c/experimental/getlines.c b/c/experimental/getlines.c index 1af42fde3..ca725fc48 100644 --- a/c/experimental/getlines.c +++ b/c/experimental/getlines.c @@ -57,7 +57,8 @@ static int read_file_mlr_getcdelim(char* filename, int do_write) { if (linelen < 0) { break; } - bc += linelen; + //bc += linelen; // available by API, but make a fair comparison + bc += strlen(line); if (do_write) { fputs(line, stdout); fputc('\n', stdout); @@ -81,7 +82,8 @@ static int read_file_mlr_getsdelim(char* filename, int do_write) { if (linelen < 0) { break; } - bc += linelen; + //bc += linelen; // available by API, but make a fair comparison + bc += strlen(line); if (do_write) { fputs(line, stdout); fputc('\n', stdout); @@ -93,7 +95,7 @@ static int read_file_mlr_getsdelim(char* filename, int do_write) { } // ================================================================ -static char* read_line_fgetc(FILE* fp, char* irs, int irs_len) { +static char* read_line_fgetc(FILE* fp, char* irs) { char* line = mlr_malloc_or_die(FIXED_LINE_LEN); char* p = line; while (TRUE) { @@ -117,12 +119,11 @@ static char* read_line_fgetc(FILE* fp, char* irs, int irs_len) { static int read_file_fgetc_fixed_len(char* filename, int do_write) { FILE* fp = fopen_or_die(filename); char* irs = "\n"; - int irs_len = strlen(irs); int bc = 0; while (TRUE) { - char* line = read_line_fgetc(fp, irs, irs_len); + char* line = read_line_fgetc(fp, irs); if (line == NULL) break; if (do_write) { @@ -137,7 +138,7 @@ static int read_file_fgetc_fixed_len(char* filename, int do_write) { } // ================================================================ -static char* read_line_getc_unlocked(FILE* fp, char* irs, int irs_len) { +static char* read_line_getc_unlocked(FILE* fp, char* irs) { char* line = mlr_malloc_or_die(FIXED_LINE_LEN); char* p = line; while (TRUE) { @@ -162,12 +163,11 @@ static char* read_line_getc_unlocked(FILE* fp, char* irs, int irs_len) { static int read_file_getc_unlocked_fixed_len(char* filename, int do_write) { FILE* fp = fopen_or_die(filename); char* irs = "\n"; - int irs_len = strlen(irs); int bc = 0; while (TRUE) { - char* line = read_line_getc_unlocked(fp, irs, irs_len); + char* line = read_line_getc_unlocked(fp, irs); if (line == NULL) break; if (do_write) { @@ -182,7 +182,7 @@ static int read_file_getc_unlocked_fixed_len(char* filename, int do_write) { } // ================================================================ -static char* read_line_getc_unlocked_psb(FILE* fp, string_builder_t* psb, char* irs, int irs_len) { +static char* read_line_getc_unlocked_psb(FILE* fp, string_builder_t* psb, char* irs) { while (TRUE) { int c = getc_unlocked(fp); if (c == EOF) { @@ -201,7 +201,6 @@ static char* read_line_getc_unlocked_psb(FILE* fp, string_builder_t* psb, char* static int read_file_getc_unlocked_psb(char* filename, int do_write) { FILE* fp = fopen_or_die(filename); char* irs = "\n"; - int irs_len = strlen(irs); int bc = 0; @@ -210,7 +209,7 @@ static int read_file_getc_unlocked_psb(char* filename, int do_write) { sb_init(&sb, STRING_BUILDER_INIT_SIZE); while (TRUE) { - char* line = read_line_getc_unlocked_psb(fp, psb, irs, irs_len); + char* line = read_line_getc_unlocked_psb(fp, psb, irs); if (line == NULL) break; if (do_write) { @@ -225,7 +224,7 @@ static int read_file_getc_unlocked_psb(char* filename, int do_write) { } // ================================================================ -static char* read_line_fgetc_psb(FILE* fp, string_builder_t* psb, char* irs, int irs_len) { +static char* read_line_fgetc_psb(FILE* fp, string_builder_t* psb, char* irs) { while (TRUE) { int c = fgetc(fp); if (c == EOF) { @@ -244,7 +243,6 @@ static char* read_line_fgetc_psb(FILE* fp, string_builder_t* psb, char* irs, int static int read_file_fgetc_psb(char* filename, int do_write) { FILE* fp = fopen_or_die(filename); char* irs = "\n"; - int irs_len = strlen(irs); string_builder_t sb; string_builder_t* psb = &sb; @@ -253,7 +251,7 @@ static int read_file_fgetc_psb(char* filename, int do_write) { int bc = 0; while (TRUE) { - char* line = read_line_fgetc_psb(fp, psb, irs, irs_len); + char* line = read_line_fgetc_psb(fp, psb, irs); if (line == NULL) break; if (do_write) { @@ -268,7 +266,7 @@ static int read_file_fgetc_psb(char* filename, int do_write) { } // ================================================================ -static char* read_line_mmap_psb(file_reader_mmap_state_t* ph, string_builder_t* psb, char* irs, int irs_len) { +static char* read_line_mmap_psb(file_reader_mmap_state_t* ph, string_builder_t* psb, char* irs) { char *p = ph->sol; while (TRUE) { if (p == ph->eof) { @@ -290,7 +288,6 @@ static char* read_line_mmap_psb(file_reader_mmap_state_t* ph, string_builder_t* static int read_file_mmap_psb(char* filename, int do_write) { file_reader_mmap_state_t* ph = file_reader_mmap_open(filename); char* irs = "\n"; - int irs_len = strlen(irs); string_builder_t sb; string_builder_t* psb = &sb; @@ -299,7 +296,7 @@ static int read_file_mmap_psb(char* filename, int do_write) { int bc = 0; while (TRUE) { - char* line = read_line_mmap_psb(ph, psb, irs, irs_len); + char* line = read_line_mmap_psb(ph, psb, irs); if (line == NULL) break; if (do_write) {