From: John B. <jj...@be...> - 2000-08-13 13:48:27
Attachments:
saveload.tar.gz
|
Dear Graham and List Folk, Hello. I was playing around with Graham's idea of dumping the parsed ASN.1 tree to a file and loading it again. The motivation of this is to reduce startup time for Net::LDAP and so improve interactive response of 'one shot' commands - following Graham's observation that a significant proportion of the startup time was parsing the ASN.1 definitions. I managed to get the following results: [Current code] time perl -MNet::LDAP::ASN -e exit Fastest time 0.55s [Using Data::Dumper] time perl -I. -MNet::LDAP::ASN -e exit Fastest time 0.45s [Using 'custom' loader/unloader] time perl -I. -MNet::LDAP::ASN -e exit Fastest time 0.35s On my system the equivalent timings for Net::LDAP are: time perl -MNet::LDAP -e exit Fastest time 0.76s [Using 'custom' loader/unloader] time perl -I. -MNet::LDAP -e exit Fasttest time 0.55s So I didn't manage a dramatic speedup but there may be more mileage in tweaking it further (this is the first 'working' version...hence unoptimised). The code isn't 100% yet (some 'undefined value' warnings when running) but I managed to run a Tk LDAP browser using it and do some searches so it seems sensible as a proof of concept. The dump file format is subject to refinement too. I'd appreciate anyone's views as to whether this is something worth pursuing further. The Net::LDAP startup time does annoy me a little (typing 'phone bloggs' and getting their phone no. is only cool if it is quicker than turning away from the keyboard to look at the phone list behind you :-). Attached is a gzip'd tar containing a patch against Convert::ASN1.pm (v0.20, not the current CVS I'm afraid), a 'Net::LDAP::ASN2.pm' and an 'ldap.asn'. To force your Net::LDAP to use the new loader, replace your Net::LDAP::ASN.pm with the ASN2.pm and patch your Convert::ASN. Then you'll need the 'ldap.asn' in the current directory of your app when it runs. regards, jb |
From: Graham B. <gb...@po...> - 2000-08-14 10:48:16
|
John, this looks good and is certainly what I was thinking of. Could you describe the format of your custom format. I cannot see exactly how it works. For example search for 0x80cadc0, which I think if supposed to be the root of the Filter, but I cannot see what I think is a definition. Graham. |
From: John B. <jj...@be...> - 2000-08-14 20:53:52
|
On Mon, 14 Aug 2000, Graham Barr wrote: > John, this looks good and is certainly what I was thinking of. > > Could you describe the format of your custom format. I cannot see > exactly how it works. Fair enough. Its pretty convoluted since I didn't initially allow for the fact that the ASN.1 tree contains nodes which don't refer to tags (e.g. the LDAPEntry definition, which is COMPONENTS OF). This was hacked on later. I may need to rethink the whole thing :-) > For example search for 0x80cadc0, which I think > if supposed to be the root of the Filter, but I cannot see what I > think is a definition. That first (only) reference is a definition. Pointers are defined in one of two ways. By occurring as the first or last item on a line. If the first item in a line is not a '_' it defines a new array reference (blessed to be a Convert::ASN::parser). The contents of the referenced array are the rest of the line. These correspond to the "cTAG cTYPE cVAR cLOOP cOPT cCHILD". If the last item in a line is not a '_' it either creates a new array reference or adds to an existing one. In either case the reference is not blessed and is stored as the cCHILD member of the containing array. The contents of the referenced array are the nodes created by the following lines (up to the next line with just a '-' on it). The indentation is not used for parsing - but makes it more readable. If the last item in a line is a '_' then the line was simple (no children) and we stop recursion. The case I initially didn't catch is the one now represented by a leading '_'. This refers to a node in the tree which is a straight array ref, not blessed to Convert::ASN::parser There are currently bugs relating to the fact that for the cTAG member, I map '' -> '0' -> (chr 0). I also introduce missing array elts as undef - but I suspect that it OK. I don't fully understand your structures, I've just tried to preserve them. Comparing the Data::Dumper output of the tree as created by the real Net::LDAP::ASN and as loaded from the file using the ->load() method show a few differences, especially in the Filter definition. However, I'm not sure that that isn't down to cosmetic reordering. I wanted initially to order the output so that the reading order contained no forward references. I then realised that this wasn't possible given the self-referential nature of e.g. Filter. Thats when I gave up trying to make it pretty :-) If you haven't found it already, the ->save() method allows you to create the .asn output. If you like the idea I'll try and clean things up. regards, jb PS. Some time ago you suggested I check out the Devel:: modules. The Devel::DProf module is cool. It shows up that Net::LDAP::Schema::_get_one_word is a significant proportion (10-20%) of a typical Schema load time. It is a one-liner used in two places, so inlining is probably a good idea... are there any perl pragma's for this yet or do we just cut and paste? |
From: Graham B. <gb...@po...> - 2000-08-15 11:02:56
|
On Mon, Aug 14, 2000 at 10:47:14PM +0100, John Berthels wrote: > PS. Some time ago you suggested I check out the Devel:: modules. The > Devel::DProf module is cool. It shows up that > Net::LDAP::Schema::_get_one_word is a significant proportion (10-20%) of a > typical Schema load time. It is a one-liner used in two places, so > inlining is probably a good idea... are there any perl pragma's for this > yet or do we just cut and paste? I have taken a hatchet to the parsing code and managed to reduce it's time taken. I replaced the nibble parser with a tokenizing parser with the following effect, approx. a 30% speedup. Each read data/schema.in 100 times [before] Total Elapsed Time = 4.576090 Seconds User+System Time = 4.576090 Seconds Exclusive Times %Time ExclSec CumulS #Calls sec/call Csec/c Name 32.3 1.480 1.436 29000 0.0001 0.0000 Net::LDAP::Schema::_parse_item 22.4 1.029 2.406 11000 0.0001 0.0002 Net::LDAP::Schema::_parse_value 17.9 0.820 0.803 11000 0.0001 0.0001 Net::LDAP::Schema::_fixup_entry 11.7 0.536 3.736 100 0.0054 0.0374 Net::LDAP::Schema::_parse_schema 8.96 0.410 0.479 100 0.0041 0.0048 Net::LDAP::LDIF::_read_one 1.53 0.070 0.070 100 0.0007 0.0007 Net::LDAP::Entry::add 1.53 0.070 0.080 14 0.0050 0.0057 Convert::ASN1::BEGIN 1.29 0.059 0.107 1 0.0586 0.1071 Convert::ASN1::parser::yyparse 1.09 0.050 0.048 905 0.0001 0.0001 Convert::ASN1::parser::yylex 0.87 0.040 4.499 100 0.0004 0.0450 Net::LDAP::Schema::parse 0.44 0.020 0.030 3 0.0067 0.0100 main::BEGIN 0.44 0.020 0.117 245 0.0001 0.0005 Convert::ASN1::parser::compile_one 0.44 0.020 0.020 100 0.0002 0.0002 Net::LDAP::LDIF::new 0.22 0.010 0.010 300 0.0000 0.0000 Net::LDAP::Entry::get 0.22 0.010 0.010 2 0.0050 0.0050 SelectSaver::BEGIN [after] Total Elapsed Time = 3.198119 Seconds User+System Time = 3.188119 Seconds Exclusive Times %Time ExclSec CumulS #Calls sec/call Csec/c Name 66.0 2.107 2.315 100 0.0211 0.0231 Net::LDAP::Schema::_parse_schema 13.1 0.420 0.468 100 0.0042 0.0047 Net::LDAP::LDIF::_read_one 7.21 0.230 0.208 11000 0.0000 0.0000 Net::LDAP::Schema::_fixup_entry 2.20 0.070 0.080 14 0.0050 0.0057 Convert::ASN1::BEGIN 2.13 0.068 0.106 1 0.0682 0.1062 Convert::ASN1::parser::yyparse 1.25 0.040 0.038 905 0.0000 0.0000 Convert::ASN1::parser::yylex 1.25 0.040 0.049 100 0.0004 0.0005 Net::LDAP::Entry::add 0.94 0.030 3.055 100 0.0003 0.0305 Net::LDAP::Schema::parse 0.63 0.020 0.030 4 0.0050 0.0075 main::BEGIN 0.31 0.010 0.010 100 0.0001 0.0001 Net::LDAP::LDIF::done 0.31 0.010 0.010 100 0.0001 0.0001 Net::LDAP::Entry::_build_attrs 0.31 0.010 0.010 100 0.0001 0.0001 Symbol::gensym 0.31 0.010 0.010 2 0.0050 0.0050 SelectSaver::BEGIN 0.31 0.010 0.010 2 0.0050 0.0050 lib::BEGIN 0.31 0.010 0.214 103 0.0001 0.0021 Net::LDAP::Entry::BEGIN Graham. |
From: Graham B. <gb...@po...> - 2000-08-15 16:18:55
|
On Mon, Aug 14, 2000 at 10:47:14PM +0100, John Berthels wrote: > The case I initially didn't catch is the one now represented by a leading > '_'. This refers to a node in the tree which is a straight array ref, not > blessed to Convert::ASN::parser The fact that some are blessed does not mean anything. I use it during a post-parse stage to mean "been here". > There are currently bugs relating to the fact that for the cTAG member, I > map '' -> '0' -> (chr 0). That is is a problem. The cTAG element is the tag packed. I would suggest outputing it as unpack("H*",$cTAG) and read it back with pack("H*",$input) that way '' will be out put as ,, and '0' will be output as ,30, > I also introduce missing array elts as undef - That will not be a problem. > If you like the idea I'll try and clean things up. Sure. > PS. Some time ago you suggested I check out the Devel:: modules. The > Devel::DProf module is cool. It shows up that > Net::LDAP::Schema::_get_one_word is a significant proportion (10-20%) of a > typical Schema load time. It is a one-liner used in two places, so > inlining is probably a good idea... are there any perl pragma's for this > yet or do we just cut and paste? Well its a bit more than one line, its 2 :) There is no way to get perl to inline yet, so we will have to copy the code. Graham. |
From: John B. <jj...@be...> - 2000-08-20 22:37:59
Attachments:
conv-asn-saveload-0.20-v2.patch
|
OK - here is the second cut of the Convert::ASN1 save/load. I've reworked it in the light of Graham's comments. (Including those on recursion :-) Attached is a gzipped tar with the patch for Convert/ASN1.pm, an 'ldap.asn' file and an 'Net/LDAP/ASN.pm.new' which you need to use the ->load method when you start Net::LDAP. Patch is against version 0.07 of ASN1.pm (i.e. the current CPAN rather than the current CVS). The fastest load time for this I've seen is 0.31s, against a fastest load time of 0.55s for the current code. (This is looking at 'time -MNet::LDAP::ASN -e exit'). Using '-d:DProf' and looking at the CumulS column: The new ->load function takes ~0.07s and the old ->prepare function takes ~0.31s. These measurements seem to agree with the 'time' method and suggest a nice speedup. The format of the ldap.asn file should now be more obvious to humans. The empty tag bug should be fixed. It tests out OK with my test app here. YMMV. regards, jb |
From: John B. <joh...@ne...> - 2000-08-21 08:36:25
|
> Attached is a gzipped tar with the patch for Convert/ASN1.pm, an > 'ldap.asn' file and an 'Net/LDAP/ASN.pm.new' which you need to use the > ->load method when you start Net::LDAP. Patch is against version 0.07 of > ASN1.pm (i.e. the current CPAN rather than the current CVS). Err. no. Attached was just the patch itself. To generate the 'ldap.asn' file you'll need to play around a little and put a ->save() somewhere. Sorry about that. regards, jb |
From: Graham B. <gb...@po...> - 2000-08-24 17:07:30
|
This is great and I will add it in. However given Benchmark: timing 1000 iterations of load, parse, storable... load: 28 wallclock secs (28.29 usr + 0.19 sys = 28.48 CPU) parse: 123 wallclock secs (122.90 usr + 0.06 sys = 122.96 CPU) storable: 4 wallclock secs ( 3.97 usr + 0.16 sys = 4.13 CPU) It seems like it could be worth using storable. But is takes 0.02 to loas Storable, so the benefit is lost. However I am thinking, is this really worth it ? According to the above the parse is taking 0.122 seconds and the load takes 0.028 a saving of 0.1 seconds at startup time. Even you times only said it saved 0.24, is this extra maintenance (ie any change to the parse tree will need to be accounted for) really woth that extra 0.24 seconds ? Graham. On Mon, Aug 21, 2000 at 12:44:05AM +0100, John Berthels wrote: > > OK - here is the second cut of the Convert::ASN1 save/load. I've reworked > it in the light of Graham's comments. (Including those on recursion :-) > > Attached is a gzipped tar with the patch for Convert/ASN1.pm, an > 'ldap.asn' file and an 'Net/LDAP/ASN.pm.new' which you need to use the > ->load method when you start Net::LDAP. Patch is against version 0.07 of > ASN1.pm (i.e. the current CPAN rather than the current CVS). > > The fastest load time for this I've seen is 0.31s, against a fastest load > time of 0.55s for the current code. (This is looking at 'time > -MNet::LDAP::ASN -e exit'). > > Using '-d:DProf' and looking at the CumulS column: The new ->load function > takes ~0.07s and the old ->prepare function takes ~0.31s. These > measurements seem to agree with the 'time' method and suggest a nice > speedup. > > The format of the ldap.asn file should now be more obvious to humans. > > The empty tag bug should be fixed. > > It tests out OK with my test app here. YMMV. > > regards, > > jb > --- /usr/lib/perl5/site_perl/5.005/Convert/ASN1.pm Tue May 30 10:00:01 2000 > +++ Convert/ASN1.pm Mon Aug 21 00:09:19 2000 > @@ -7,6 +7,7 @@ > use strict; > use vars qw($VERSION @ISA @EXPORT_OK %EXPORT_TAGS @opParts @opName $AUTOLOAD); > use Exporter; > +#se Data::Dumper; > > BEGIN { > @ISA = qw(Exporter); > @@ -138,6 +139,219 @@ > $self->{tree} = _pack_struct($tree); > $self->{script} = (values %$tree)[0]; > $self; > +} > + > +# > +# Here we provide for dumping and reloading the ASN.1 tree from a file. > +# Essentially this allows us to skip the 'prepare' method by doing a 'load' > +# instead. > +# > +sub save { > + my $self = shift; > + my $file = shift; > + > + open( OUT, "> $file" ) or die( "Can't write to file [$file] : $!" ); > +# Data::Dumper version > +# print OUT Data::Dumper->Dump( [$self], [ '$asn' ] ); > +# close OUT; > +# return 1; > + my $tree = $self->{tree}; > + my %seen; # Avoid writing same ref twice > + my( $name, $base ); > + while( ( $name, $base ) = each %$tree ) { > + # Start a new ASN.1 type definition > + print OUT $name, ":", _ref_to_name( $base ), "\n"; > + foreach my $op ( @$base ) { > + _write_op_tree( \*OUT, $base, \%seen ) > + or die( "Unable to write op for [$name] : $!" ); > + } > + print OUT "\n"; # End of type > + } > + close OUT; > + > + return 1; > +} > + > +# > +# We have two kinds of reference. An 'op' which has an ASN.1 tag > +# and a plain array reference (AR). OPs may have an AR references as their > +# cCHILD parameter and ARs contain OPs. > +# Since we label each reference, the order we write out in does not matter. > +# > +sub _write_op_tree { > + my $fh = shift; > + my $base = shift; > + my $seen = shift; > + > + my @ars = ( $base ); # List of array refs to write > + my @ops; # List of ops to write > + while( @ars || @ops) { > + if( @ars ) { > + my $array = shift @ars; > + next if $seen->{$array}; # Already written, so skip > + _write_ar( $fh, $array ) # Do the output > + or return undef; > + $seen->{$array}++; # Remember > + push @ops, @$array; # And remember the contents > + } > + if( @ops ) { > + my $op = shift @ops; > + next if $seen->{$op}; > + _write_op( $fh, $op ) > + or return undef; > + $seen->{$op}++; > + if( ref $op->[cCHILD] eq "ARRAY" ) { > + push @ars, $op->[cCHILD]; > + } > + } > + } > + > + return 1; > +} > + > +sub _write_ar { > + my $fh = shift; > + my $array = shift; > + > + my $name = _ref_to_name( $array ); > + print $fh $name, "["; > + print $fh join( ",", map { _ref_to_name( $_ ) } @$array ); > + print $fh "]\n"; > + > + return 1; > +} > + > + > +sub _write_op { > + my $fh = shift; > + my $op = shift; > + > +# cTAG cTYPE cVAR cLOOP cOPT cCHILD > + > + my $name = _ref_to_name( $op ); > + print $fh $name, ":"; > + my @line; > + push @line, defined( $op->[cTAG] ) > + ? ($op->[cTAG] eq '') > + ? '' > + : ord $op->[cTAG] > + : "_"; > + push @line, $op->[cTYPE]; > + push @line, defined( $op->[cVAR] ) ? $op->[cVAR] : "_"; > + push @line, defined( $op->[cLOOP] ) ? $op->[cLOOP] : "_"; > + push @line, defined( $op->[cOPT] ) ? $op->[cOPT] : "_"; > + > + if( ref $op->[cCHILD] eq "ARRAY" ) { > + push @line, _ref_to_name( $op->[cCHILD] ); > + } > + else { > + push @line, "_"; > + } > + print $fh join( ",", @line ), "\n"; > + > + return 1; > +} > + > +# We could use nice names here. At the moment, just use address. This > +# is techically naughty since we assume something about their structure. > +# A cleaner way would be to remember which we have seen and assign unique > +# ids. > +sub _ref_to_name { > + my $name = shift; > + return "_" unless defined $name; > + $name =~ s/^.*\(//; > + $name =~ s/\).*$//; > + return $name; > +} > + > +# > +# Read in the output of a 'save'. Here we overwrite the 'tree' and the > +# 'script' in $self, just as if prepare had been called. Could probably > +# share a few lines with ->prepare here and/or add some automagic to the > +# args its args to take a saved file instead. > +# > +sub load { > + my $self = shift; > + my $file = shift; > + > +# Data::Dumper version > +# my $asn; > +# require $file or die( "Can't load file : $!" ); > +# $self = $asn; > +# return 1; > + > + my $tree = {}; # This is the structure we are building > + my %refs; # And here we map ID -> array reference > + > + open( IN, "< $file" ) or die( "Unable to open file [$file] : $!" ); > + > + my $at_start = 1; > + my $line; > + while( $line = <IN> ) { > + chomp $line; > + > + unless( $line ) { > + $at_start = 1; > + next; > + } > + > + if( $at_start ) { > + my( $name, $id ) = split( /:/, $line, 2 ); > + die( "Invalid record start line [$line]" ) unless $name && $id; > + $tree->{$name} = _id2ref( $id, \%refs ); > + $at_start = 0; > + next; > + } > + > + my( $id, $line ) = split( /[:[]/, $line, 2 ); > + die( "Invalid input line [$line]" ) unless $id && $line; > + > + # Get or create + store ref for this ID > + my $ref = _id2ref( $id, \%refs ); > + > + if( $line =~ /]$/ ) { > + # This is an array of IDs. Store in @$ref. > + # Create them if neccesary. > + $line =~ s/\]$//; > + my @ids = split( /,/, $line ); > + foreach my $child_id ( @ids ) { > + die( "Invalid id [$line]" ) unless $child_id; > + push @$ref, _id2ref( $child_id, \%refs ); > + } > + } > + else { > + # This is a tag definition - cTAG cTYPE cVAR cLOOP cOPT cCHILD > +# $line =~ s/^.//; > + > + my @bits = split( /,/, $line ); > + my $tag = shift @bits; > + push @$ref, ($tag eq "_") ? undef > + : ($tag eq '') ? '' > + : chr $tag; > + my $val; > + while( $val = shift @bits ) { > + push @$ref, ($val eq "_") ? undef : $val; > + } > + # > + # If last val was an id. Map it to a ref. > + # > + $ref->[cCHILD] = _id2ref( $ref->[cCHILD], \%refs ) > + if( defined $ref->[cCHILD] ); > + } > + } > + close IN; > + > + $self->{tree} = $tree; > + $self->{script} = (values %$tree)[0]; > + > + return 1; > +} > + > +sub _id2ref { > + my $id = shift; > + my $refs = shift; > + > + return ( $refs->{$id} ||= [] ); > } > > # In XS the will convert the tree between perl and C structs |
From: John B. <joh...@ne...> - 2000-08-25 13:21:57
|
On Thu, 24 Aug 2000, Graham Barr wrote: > However I am thinking, is this really worth it ? According to the > above the parse is taking 0.122 seconds and the load takes 0.028 a > saving of 0.1 seconds at startup time. Even you times only said it > saved 0.24, is this extra maintenance (ie any change to the parse tree > will need to be accounted for) really woth that extra 0.24 seconds ? I'm not sure. On my home box I found it took around 60% of the startup time (for Net::LDAP::ASN) that it did previously. It is only of use (if at all) for those one-shot applications where command latency is an issue. This is something I've noticed in the past being a little annoying. For me, this 'feels' snappier when I run one-shot commands which use Net::LDAP. The potential maintenance burden is not something to be taken lightly, I agree. It might be possible to code a little more defensively against ASN.1 tree format changes and/or have a fallback method to parse the definition if the load fails. I don't know if that would help or not. What do other people think? I believe that there was some discussion a few weeks back about Net::LDAP seeming much slower than C APIs when used as a command line tool which I believe boiled down to startup times. Would anyone involved in that care to try this patch and see if there is a significant improvement for their application? If the current format of the patch is problematic to employ, mail me directly and I'll provide a more easily tried version. regards, jb |
From: Kurt D. Z. <Ku...@Op...> - 2000-08-25 22:19:00
|
At 02:21 PM 8/25/00 +0100, John Berthels wrote: >What do other people think? Well, since you asked... I think it's a little premature to be optimizing net::LDAP. I'd prefer a complete and correct API over an optimized one any day. And, when "completeness" is declared, the optimizations that will be critical to me will improve overall runtime performance (such as Convert::ASN1 optimizations). As you've noted, start up time only matters for small "one-shot" applications. I would argue that such applications which were also time critical would be reimplemented using a language/API that compiled native executable. For my uses of net::ldap, start up time rarely factors in. Even for small "one-shot" applications, they are usually just a little script I just whipped up to fulfill a requirement of the moment. The startup cost of net::ldap (and perl in general) is easily offset by other costs (such as my time whipping the thing together). And if this script ever became time critical, I'd write the thing in C to avoid the perl overhead or use another approach (such, as in the web space, changing from CGI to mod_perl). Now, I'd like to see the completed net::LDAP optimized. I believe net::LDAP can be optimized to operate at comparable run time speeds of other Perl LDAP modules... and startup time (if it's really an issue) could be improved as well... but completeness and correctness first. Kurt |
From: John B. <jj...@be...> - 2000-08-26 14:35:53
Attachments:
net-ldap-entrydiff-v0.20.patch.gz
|
Hi. Attached is a gzipped patch which attempts to implement a ->diff() method for Net::LDAP::Entry. There is a little POD in the patch too. Patch is against vanilla 0.20. The intention is that you can do things like: my $old_entry = ...read from LDAP server my $new_entry = ...read from LDIF my $diff_entry = $old_entry->diff( $new_entry ); $diff_entry->update( $ldap_server ); The only benefit of this over an add/delete sequence is that it should occur as one LDAP operation. Hence a power failure shouldn't leave you with the potential embarassement of an entry missing from the server. No attempt is made to reconcile changes in the DN - it just fails. regards, jb |
From: Graham B. <gb...@po...> - 2000-09-01 16:47:17
|
It looks good. I'll assume you tested it :) Just one point. We might want to allow the attribute names to be passed and default to the union of the attributes in $new and $old if not specified. Graham. On Sat, Aug 26, 2000 at 04:42:22PM +0100, John Berthels wrote: > Hi. > > Attached is a gzipped patch which attempts to implement a ->diff() method > for Net::LDAP::Entry. There is a little POD in the patch too. Patch is > against vanilla 0.20. > > The intention is that you can do things like: > > my $old_entry = ...read from LDAP server > my $new_entry = ...read from LDIF > > my $diff_entry = $old_entry->diff( $new_entry ); > $diff_entry->update( $ldap_server ); > > The only benefit of this over an add/delete sequence is that it should > occur as one LDAP operation. Hence a power failure shouldn't leave you > with the potential embarassement of an entry missing from the server. > > No attempt is made to reconcile changes in the DN - it just fails. > > regards, > > jb > |
From: John B. <jj...@be...> - 2000-09-01 21:18:03
|
On Fri, 1 Sep 2000, Graham Barr wrote: > It looks good. I'll assume you tested it :) "To some degree". On that note, I guess it would be a good idea to submit patch+pod+regression test(s) together? I've not looked at the module packaging and testing side of things before. I'll see if any of the docs make sense to me. > Just one point. We might want to allow the attribute names to be passed > and default to the union of the attributes in $new and $old if > not specified. I don't see wht not. But would that mean deleting any unmentioned attributes or leaving unchanged? (Or are both potentially useful and selectable? Or is that making one func do too much?) jb |
From: Graham B. <gb...@po...> - 2000-09-01 21:26:44
|
On Fri, Sep 01, 2000 at 11:25:25PM +0100, John Berthels wrote: > > Just one point. We might want to allow the attribute names to be passed > > and default to the union of the attributes in $new and $old if > > not specified. > > I don't see wht not. But would that mean deleting any unmentioned > attributes or leaving unchanged? (Or are both potentially useful and > selectable? Or is that making one func do too much?) I would say the passed list would be the list of attributes to consider. ie ignore/don't change all others. Graham. |