Skip Menu |

This queue is for tickets about the Text-CSV_XS CPAN distribution.

Report information
The Basics
Id: 42517
Status: resolved
Priority: 0/
Queue: Text-CSV_XS

People
Owner: Nobody in particular
Requestors: gfuji [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Wishlist
Broken in: (no value)
Fixed in: 0.61



Subject: A little performance improvement
Hello, I have written a patch to use PERL_NO_GET_CONTEXT for efficiency, which makes CSV_XS a little faster, especially some platforms, i.g. Cygwin with ithreads. The patch includes some other changes: some unused variables removed and using newSVpvs() instead of newSVpv() in . Regards, -- Goro Fuji (GFUJI at CPAN.org)
Subject: CSV_XS.diff
*** CSV_XS.xs~ Sun Jan 18 19:43:28 2009 --- CSV_XS.xs Sun Jan 18 23:43:14 2009 *************** *** 3,9 **** * This program is free software; you can redistribute it and/or * modify it under the same terms as Perl itself. */ ! #include <EXTERN.h> #include <perl.h> #include <XSUB.h> --- 3,9 ---- * This program is free software; you can redistribute it and/or * modify it under the same terms as Perl itself. */ ! #define PERL_NO_GET_CONTEXT #include <EXTERN.h> #include <perl.h> #include <XSUB.h> *************** *** 172,183 **** unless (io_handle_loaded) { \ ENTER; \ load_module (PERL_LOADMOD_NOIMPORT, \ ! newSVpv ("IO::Handle", 0), NULL, NULL, NULL); \ LEAVE; \ io_handle_loaded = 1; \ } ! static SV *SvDiag (int xse) { int i = 0; SV *err; --- 172,186 ---- unless (io_handle_loaded) { \ ENTER; \ load_module (PERL_LOADMOD_NOIMPORT, \ ! newSVpvs ("IO::Handle"), NULL, NULL, NULL); \ LEAVE; \ io_handle_loaded = 1; \ } ! static SV* m_getline; ! ! #define SvDiag(xse) cx_SvDiag(aTHX_ xse) ! static SV *cx_SvDiag (pTHX_ int xse) { int i = 0; SV *err; *************** *** 191,199 **** return (err); } /* SvDiag */ ! static SV *SetDiag (csv_t *csv, int xse) { - int i = 0; SV *err = SvDiag (xse); if (err) { --- 194,202 ---- return (err); } /* SvDiag */ ! #define SetDiag(csv, xse) cx_SetDiag(aTHX_ csv, xse) ! static SV *cx_SetDiag (pTHX_ csv_t *csv, int xse) { SV *err = SvDiag (xse); if (err) { *************** *** 206,212 **** return (err); } /* SetDiag */ ! static void SetupCsv (csv_t *csv, HV *self) { SV **svp; STRLEN len; --- 209,216 ---- return (err); } /* SetDiag */ ! #define SetupCsv(csv, self) cx_SetupCsv(aTHX_ csv, self) ! static void cx_SetupCsv (pTHX_ csv_t *csv, HV *self) { SV **svp; STRLEN len; *************** *** 355,361 **** csv->used = 0; } /* SetupCsv */ ! static int Print (csv_t *csv, SV *dst) { int result; --- 359,366 ---- csv->used = 0; } /* SetupCsv */ ! #define Print(csv, dst) cx_Print(aTHX_ csv, dst) ! static int cx_Print (pTHX_ csv_t *csv, SV *dst) { int result; *************** *** 399,405 **** /* Should be extended for EBCDIC ? */ #define is_csv_binary(ch) ((ch < CH_SPACE || ch >= CH_DEL) && ch != CH_TAB) ! static int Combine (csv_t *csv, SV *dst, AV *fields) { int i; --- 404,411 ---- /* Should be extended for EBCDIC ? */ #define is_csv_binary(ch) ((ch < CH_SPACE || ch >= CH_DEL) && ch != CH_TAB) ! #define Combine(csv, dst, fields) cx_Combine(aTHX_ csv, dst, fields) ! static int cx_Combine (pTHX_ csv_t *csv, SV *dst, AV *fields) { int i; *************** *** 495,501 **** #if MAINT_DEBUG static char str_parsed[40]; #endif ! static void ParseError (csv_t *csv, int xse, int pos) { hv_store (csv->self, "_ERROR_POS", 10, newSViv (pos), 0); if (csv->tmp) { --- 501,509 ---- #if MAINT_DEBUG static char str_parsed[40]; #endif ! ! #define ParseError(csv, xse, pos) cx_ParseError(aTHX_ csv, xse, pos) ! static void cx_ParseError (pTHX_ csv_t *csv, int xse, int pos) { hv_store (csv->self, "_ERROR_POS", 10, newSViv (pos), 0); if (csv->tmp) { *************** *** 505,511 **** (void)SetDiag (csv, xse); } /* ParseError */ ! static int CsvGet (csv_t *csv, SV *src) { unless (csv->useIO) return EOF; --- 513,520 ---- (void)SetDiag (csv, xse); } /* ParseError */ ! #define CsvGet(csv, src) cx_CsvGet(aTHX_ csv, src) ! static int cx_CsvGet (pTHX_ csv_t *csv, SV *src) { unless (csv->useIO) return EOF; *************** *** 520,526 **** EXTEND (sp, 1); PUSHs (src); PUTBACK; ! result = call_method ("getline", G_SCALAR); SPAGAIN; csv->tmp = result ? POPs : NULL; PUTBACK; --- 529,535 ---- EXTEND (sp, 1); PUSHs (src); PUTBACK; ! result = call_sv (m_getline, G_SCALAR | G_METHOD); SPAGAIN; csv->tmp = result ? POPs : NULL; PUTBACK; *************** *** 607,613 **** } #endif ! static void strip_trail_whitespace (SV *sv) { STRLEN len; char *s = SvPV (sv, len); --- 616,623 ---- } #endif ! #define strip_trail_whitespace(sv) cx_strip_trail_whitespace(aTHX_ sv) ! static void cx_strip_trail_whitespace (pTHX_ SV *sv) { STRLEN len; char *s = SvPV (sv, len); *************** *** 618,624 **** SvCUR_set (sv, len); } /* strip_trail_whitespace */ ! static SV *bound_field (csv_t *csv, int i) { SV *sv = csv->bound; AV *av; --- 628,635 ---- SvCUR_set (sv, len); } /* strip_trail_whitespace */ ! #define bound_field(csv, i) cx_bound_field(aTHX_ csv, i) ! static SV *cx_bound_field (pTHX_ csv_t *csv, int i) { SV *sv = csv->bound; AV *av; *************** *** 655,661 **** f = 0; \ } ! static int Parse (csv_t *csv, SV *src, AV *fields, AV *fflags) { int c, f = 0; int c_ungetc = EOF; --- 666,673 ---- f = 0; \ } ! #define Parse(csv, src, fields, fflags) cx_Parse(aTHX_ csv, src, fields, fflags) ! static int cx_Parse (pTHX_ csv_t *csv, SV *src, AV *fields, AV *fflags) { int c, f = 0; int c_ungetc = EOF; *************** *** 1077,1083 **** return TRUE; } /* Parse */ ! static int xsParse (HV *hv, AV *av, AV *avf, SV *src, bool useIO) { csv_t csv; int result; --- 1089,1096 ---- return TRUE; } /* Parse */ ! #define xsParse(hv, av, avf, src, useIO) cx_xsParse(aTHX_ hv, av, avf, src, useIO) ! static int cx_xsParse (pTHX_ HV *hv, AV *av, AV *avf, SV *src, bool useIO) { csv_t csv; int result; *************** *** 1135,1141 **** return result; } /* xsParse */ ! static int xsCombine (HV *hv, AV *av, SV *io, bool useIO) { csv_t csv; int result; --- 1148,1155 ---- return result; } /* xsParse */ ! #define xsCombine(hv, av, io, useIO) cx_xsCombine(aTHX_ hv, av, io, useIO) ! static int cx_xsCombine (pTHX_ HV *hv, AV *av, SV *io, bool useIO) { csv_t csv; int result; *************** *** 1160,1166 **** PROTOTYPES: DISABLE ! SV* SetDiag (self, xse) SV *self int xse --- 1174,1183 ---- PROTOTYPES: DISABLE ! BOOT: ! m_getline = newSVpvs("getline"); ! ! void SetDiag (self, xse) SV *self int xse *************** *** 1179,1185 **** XSRETURN (1); /* XS SetDiag */ ! SV* Combine (self, dst, fields, useIO) SV *self SV *dst --- 1196,1202 ---- XSRETURN (1); /* XS SetDiag */ ! void Combine (self, dst, fields, useIO) SV *self SV *dst *************** *** 1196,1202 **** XSRETURN (1); /* XS Combine */ ! SV* Parse (self, src, fields, fflags) SV *self SV *src --- 1213,1219 ---- XSRETURN (1); /* XS Combine */ ! void Parse (self, src, fields, fflags) SV *self SV *src
Subject: Re: [rt.cpan.org #42517] A little performance improvement
Date: Mon, 19 Jan 2009 12:21:39 +0100
To: bug-Text-CSV_XS [...] rt.cpan.org
From: "H.Merijn Brand" <h.m.brand [...] xs4all.nl>
On Mon, 19 Jan 2009 02:38:37 -0500, "Goro Fuji via RT" <bug-Text-CSV_XS@rt.cpan.org> wrote: Show quoted text
> I have written a patch to use PERL_NO_GET_CONTEXT for efficiency, which > makes CSV_XS a little faster, especially some platforms, i.g. Cygwin > with ithreads.
I will have a look later. Point is I never use threaded perl *because* it is slower than non-threaded perl. I /do/ however test Text::CSV_XS on threaded perl, and it still works. I certainly do not want to loose performance on non-threaded perl Show quoted text
> The patch includes some other changes: some unused variables removed and > using newSVpvs () instead of newSVpv () in .
Those are unchanged leftovers from older days or yank/put usages. Done. Thanks, looks cleaner indeed. I already used newSVpvs () somewhere else, so there should be no portability issues. I like this change: ! result = call_method ("getline", G_SCALAR); --- 529,535 ---- ! result = call_sv (m_getline, G_SCALAR | G_METHOD); Nice optimisation indeed, but I cannot use that, as it doesn't compile for 5.00504: CSV_XS.xs: In function ‘CsvGet’: CSV_XS.xs:523: error: ‘G_METHOD’ undeclared (first use in this function) I wonder why you didn't optimize the "print" version too there. Part 1 is now in: http://repo.or.cz/w/Text-CSV_XS.git http://repo.or.cz/w/Text-CSV_XS.git?a=commit;h=e8a1b89c39fb539b781deb6e32d210abdc5566cf I'll have a go at the thread stuff later. -- H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/ using & porting perl 5.6.2, 5.8.x, 5.10.x, 5.11.x on HP-UX 10.20, 11.00, 11.11, 11.23, and 11.31, SuSE 10.1, 10.3, and 11.0, AIX 5.2, and Cygwin. http://mirrors.develooper.com/hpux/ http://www.test-smoke.org/ http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/
Subject: Re: [rt.cpan.org #42517] A little performance improvement
Date: Tue, 20 Jan 2009 17:37:33 +0900
To: bug-Text-CSV_XS [...] rt.cpan.org
From: Goro Fuji <gfuji [...] cpan.org>
2009/1/19 h.m.brand@xs4all.nl via RT <bug-Text-CSV_XS@rt.cpan.org>: Show quoted text
> <URL: http://rt.cpan.org/Ticket/Display.html?id=42517 > > > On Mon, 19 Jan 2009 02:38:37 -0500, "Goro Fuji via RT" > <bug-Text-CSV_XS@rt.cpan.org> wrote: >
>> I have written a patch to use PERL_NO_GET_CONTEXT for efficiency, which >> makes CSV_XS a little faster, especially some platforms, i.g. Cygwin >> with ithreads.
> > I will have a look later. Point is I never use threaded perl *because* > it is slower than non-threaded perl. I /do/ however test Text::CSV_XS > on threaded perl, and it still works. > > I certainly do not want to loose performance on non-threaded perl >
>> The patch includes some other changes: some unused variables removed and >> using newSVpvs () instead of newSVpv () in .
> > Those are unchanged leftovers from older days or yank/put usages. Done. > Thanks, looks cleaner indeed. I already used newSVpvs () somewhere > else, so there should be no portability issues. > > I like this change: > > ! result = call_method ("getline", G_SCALAR); > --- 529,535 ---- > ! result = call_sv (m_getline, G_SCALAR | G_METHOD); > > Nice optimisation indeed, but I cannot use that, as it doesn't compile > for 5.00504:
I see. v5.005_04 of perl_call_method() is: I32 perl_call_method(char *methname, I32 flags) /* name of the subroutine */ /* See G_* flags in cop.h */ { dSP; OP myop; if (!PL_op) PL_op = &myop; XPUSHs(sv_2mortal(newSVpv(methname,0))); PUTBACK; pp_method(ARGS); if(PL_op == &myop) PL_op = Nullop; return perl_call_sv(*PL_stack_sp--, flags); } To remove newSVpv(methname, 0), it is possible to copy this function, but I don't know it is worth doing so. Show quoted text
> CSV_XS.xs: In function 'CsvGet': > CSV_XS.xs:523: error: 'G_METHOD' undeclared (first use in this function) > > I wonder why you didn't optimize the "print" version too there.
I thought that files ware written a few times while read many times, but the optimization of "print" is also useful, although its effect is less than that of "getline". Show quoted text
Thanks, -- Goro Fuji (藤 吾郎)
Subject: Re: [rt.cpan.org #42517] A little performance improvement
Date: Fri, 23 Jan 2009 08:03:14 +0100
To: bug-Text-CSV_XS [...] rt.cpan.org
From: "H.Merijn Brand" <h.m.brand [...] xs4all.nl>
On Tue, 20 Jan 2009 03:37:52 -0500, "Goro Fuji via RT" <bug-Text-CSV_XS@rt.cpan.org> wrote: Show quoted text
> > I like this change: > > > > ! result = call_method ("getline", G_SCALAR); > > --- 529,535 ---- > > ! result = call_sv (m_getline, G_SCALAR | G_METHOD); > > > > Nice optimisation indeed, but I cannot use that, as it doesn't compile > > for 5.00504:
> > I see. v5.005_04 of perl_call_method() is: > I32 > perl_call_method(char *methname, I32 flags) > /* name of the subroutine */ > /* See G_* flags in cop.h */ > { > dSP; > OP myop; > if (!PL_op) > PL_op = &myop; > XPUSHs(sv_2mortal(newSVpv(methname,0))); > PUTBACK; > pp_method(ARGS); > if(PL_op == &myop) > PL_op = Nullop; > return perl_call_sv(*PL_stack_sp--, flags); > } > > To remove newSVpv(methname, 0), it is possible to copy this function, > but I don't know it is worth doing so. >
> > CSV_XS.xs: In function 'CsvGet': > > CSV_XS.xs:523: error: 'G_METHOD' undeclared (first use in this function)
It is so nice to know all these wonderful persons in the perl community :) Devel::PPPort will release a new version soon, which includes support for G_METHOD, so both call_method ()'s are now call_sv ()'s http://repo.or.cz/w/Text-CSV_XS.git http://repo.or.cz/w/Text-CSV_XS.git?a=commitdiff;h=5ea2c4d1123003d347d243ea69d37a98a093b90c or clone if you want to have all of it actual $ git clone http://repo.or.cz/r/Text-CSV_XS.git Text-CSV_XS Thread stuff still on TODO. Show quoted text
> > I wonder why you didn't optimize the "print" version too there.
> > I thought that files ware written a few times while read many times, > but the optimization of "print" is also useful, although its effect > is less than that of "getline".
-- H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/ using & porting perl 5.6.2, 5.8.x, 5.10.x, 5.11.x on HP-UX 10.20, 11.00, 11.11, 11.23, and 11.31, SuSE 10.1, 10.3, and 11.0, AIX 5.2, and Cygwin. http://mirrors.develooper.com/hpux/ http://www.test-smoke.org/ http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/
Thread performance issues addressed in 0.60.