RE: [code-review] list scope

Already found some corrections, lol -- will post a more complete and
re-tested module soon.
My apologies for including code that had been edited and tested on a
specific circumstance without noticing what those changes would do to the
more normal case......

> -----Original Message-----
> From: code-review-ladder-admin-wool9L35kiek0rBIEb6pKdBc4/[email protected]
> [mailto:code-review-ladder-admin-wool9L35kiek0rBIEb6pKdBc4/[email protected]]On Behalf Of
> Hodges, Paul
> Sent: Friday, September 12, 2003 9:29 AM
> To: '[email protected]'; [email protected]
> Subject: RE: [code-review] list scope
> 
> 
> > -----Original Message-----
> > From: [email protected] [mailto:[email protected]]
> > Sent: Friday, September 12, 2003 4:17 AM
> > To: Hodges, Paul; code-review-ladder-wool9L35kiek0rBIEb6pKdBc4/[email protected]
> > Subject: RE: [code-review] list scope
> 
> Thanks for the response. :)
> 
> > I would have thought that showing any code would be fine. 
> > Everyone still learns from the analysis even if they never 
> have a use for
> the code.
> 
> That was my thinking, but it seemed worth asking before 
> putting deep dark
> tangles of pre-mangled code up on the list. I'll start with 
> subsections in
> the meantime.
> 
> > I've run into something similar to your problem before.
> > If you end up running various tweaked analyses on the same
> > big, hard to parse files you may get great benefit from
> > preparsing them into a faster to parse format.
> > You could use Storable for instance but for maximum speed I 
> > think you won't beat pack/unpack.
> 
> I tried several versions, but unpack seemed the best option 
> in a pure Perl
> solution.
> I think the real slowdown is the process of putting it on the object,
> though:
> 
> sub parse { no warnings; # to quiet > & == on 0A's (special 
> account types)
>     (*self,$rec) = @_;   # these are our() 
>     my $rt = substr($rec||=$self{rec},0,2) or confess "invalid usage";
>     if ($rt > 1) { # all greater than 01 is usage, most common type
>         @self{@usg_fields} = unpack $usg_layout, $rec;
>         $self{usg}         = $rec; # save the usage level raw record
>     } elsif($rt == 1) { # 01 records are line data, next most common
>         @self{@linefields} = unpack $linelayout, $rec;
>         @self{@usg}        = ();   # line cleans out previous usage
>         $self{line}        = $rec; # save the line level raw record
>     } else {       # 0[0A] is account data, only other types
>         @self{@acctfields} = unpack $acctlayout, $rec;
>         @self{@clean}      = ();   # account cleans out line 
> & usage data
>         $self{acct}        = $rec; # save account level raw record
>     }
>     # should return $rec (maybe better to explicitly return $self?)
> }
> 
> @usg_fields, @linefields, @acctfields, @usg, and @clean predefined for
> readability and hopefully a little more speed. I'm not sure 
> of the details
> on this, but if that isn't faster, I'm assuming it doesn't 
> matter. If it
> does, I probably just need to go back to the C module version 
> and tweak
> there anyway.
> 
> > It's a bit of work to split your process into a parse and 
> dump phase,
> > followed by an undump and analyse phase but if you end up 
> reanalysing even
> > once, it sounds like you'll get that time back pretty quickly.
> 
> Boss doesn't want us rewriting the file format -- we have 
> over a decade of
> legacy code reading the current format that *isn't* going to 
> be rewritten to
> a new one, so I have to say I agree with that one. :/  
> Accordingly, we're
> stuck with the file format as-is. 
> 
> The files are around a GB each, and there are 12, and we have 
> to store over
> a year's worth on limited disk space, so we keep them gzip'd. 
> We open gzcat
> pipes in our programs to decompress on the fly as an internal 
> input stream,
> and usually output to compression pipes as well, so the data 
> never touches
> the disk unzipped. All account data is stored on an account 
> record, and all
> data following until the next account record is associated with that
> account, but there's nothing on the subsequent records to say 
> so except
> sequence, so we can't do any sorts on the files. There are several
> subcategories as well. It's a structure that begs for an 
> object to store
> less granular data, and to automate cleaning that storage out 
> at the next
> account.
> 
> > Of course improving the parsing sounds like it would be 
> worth doing too.
> > In that same project I made good gains also by switching some method
> > calls inside tight loops to be function calls. Of course this is not
> > something to do without careful thought but I had some 
> methods that were 
> > never called by anything outside their own class and so I 
> just turned them
> 
> > into functions.
> 
> Done that where I could, but the main problem is that I need 
> to be able to
> tell the method I want so-and-so to be the current record 
> now, which is
> usually going to be by saying "here, parse this one".
> 
> > Finally, putting loops _inside_ methods rather than around 
> > them can be a big help. So convert from
> > 
> > foreach my $line (@lines) {
> >   push(@parsed, $parser->parse_line($line));
> > }
> > 
> > to
> > 
> > push(@parsed, $parser->parse_lines(@lines));
> > 
> > and if you need it you can just reimplement parse_line as a 
> > wrapper around parse_lines.
> 
> Absolutely -- done that, too. :)
> 
> > If @lines is enormous or is coming in from a file then 
> split them into
> > batches and you'll still see a speed up.
> > Obviously there's not much benefit if parse_lines needs 5 
> > minutes per line,
> > F
> 
> I switched from reading one record on one method call and pasing on a
> subcall to reading a MB and splitting it into an array or 
> records on one
> call, and having each request for another record just advance 
> through that
> array (reloading on the fly at need), but I still have to 
> parse each record
> onto the object. I did all that before posting. Nothing seems 
> to really help
> enough to matter. :(
> 
> sub blockread { my ($self,$buf) = shift;
>     my $read = read($self->{_fh},$buf,1105920) or return;   # 
> 54 bytes recs
>     push @{ $self->{_recBlockBuffer} }, split /\r\n/, $buf; # CRLF
> terminated
>     $read;
> }
> 
> sub nextrec {
>     *self   = $_[0];
>     *block  = $self{_recBlockBuffer};
>     my $ndx = $self{_block_ndx}++;
>     if ($ndx > $#block) {
>         @block = ();      # clean out the old
>         $_[0]->blockread; # read in the new
>         $ndx = 0;         # point at the fresh data
>     }
>     $rec = $self{rec} = $block[$ndx];
> }
> 
> I wanted to make that block assignment in blockread() be 
>     $self->{_recBlockBuffer} = [ split /\r\n/, $buf ];
> but there is almost certainly a few records left over from 
> any attempt to
> read an account at a time using 
> 
> sub readacct {
>     *self   = $_[0];
>     *block  = $self{_recBlockBuffer} ||= [ ];
>     my $ndx = $self{_block_ndx}      ||=  0;
>     SCAN:{ $ndx++ until not $block[$ndx] or $ndx and $block[$ndx] =~
> /^0[0A]/;
>         unless ($block[$ndx]) {
>             last SCAN unless $_[0]->blockread; # read new, bail if eof
>             redo SCAN; # continue until we have the whole account
>         }
>     }
>     $self{_recBlockBuffer} = [ @block[$ndx..$#block] ]; # 
> block sans acct
>     $self{_block_ndx}      = 0;                         # reset to top
>     [ @block[0..$ndx-1] ];                              # 
> return accountref
> }
> 
> readacct() isn't something I expect to see used often, but 
> it's been needed
> before....
> 
> 
> *****
> "The information transmitted is intended only for the person 
> or entity to
> which it is addressed and may contain confidential, 
> proprietary, and/or
> privileged material.  Any review, retransmission, 
> dissemination or other use
> of, or taking of any action in reliance upon, this 
> information by persons or
> entities other than the intended recipient is prohibited.  If 
> you received
> this in error, please contact the sender and delete the 
> material from all
> computers."
> 
> _______________________________________________
> code-review-ladder mailing list
> code-review-ladder-wool9L35kiek0rBIEb6pKdBc4/[email protected]
> http://lists.netthink.co.uk/listinfo/code-review-ladder
> 


*****
"The information transmitted is intended only for the person or entity to
which it is addressed and may contain confidential, proprietary, and/or
privileged material.  Any review, retransmission, dissemination or other use
of, or taking of any action in reliance upon, this information by persons or
entities other than the intended recipient is prohibited.  If you received
this in error, please contact the sender and delete the material from all
computers."