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

Report information
The Basics
Id:
42517
Status:
resolved
Priority:
Low/Low
Queue:

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

BugTracker
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.


This service runs on Request Tracker, is sponsored by The Perl Foundation, and maintained by Best Practical Solutions.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.