On Tue, Apr 29, 2008 at 01:56:26AM +0100, Rudolf Lippan wrote: > > On Mon, 28 Apr 2008 23:51:09 +0100, Tim Bunce <[EMAIL PROTECTED]> wrote: > > On Mon, Apr 28, 2008 at 03:24:10PM +0100, Rudolf Lippan wrote: > > >> Oh, And is there a way to attach a string to an SV w/o copying it? > > > > Not sure what you mean by "attach a string to an SV". I'll guess you > > mean append. In which case, yes, there are lots of ways: > > Lets say I have a char *result. If I newSVpvn(*result, strlen(result)), > *result is copied and I then have to free result... Is there a way to > create a new SV without that copy?
Ah. No, none that would let you return the SV to the caller, I believe. > >> +static int > >> +_cmp_number (val1, val2) > > > > Seeing this now I wonder if it might be better to have _sort_hash_keys > > create an array of UV in parallel with an array of char* and then qsort > > the array of UV if all the keys were valid numbers. > > There would still need to be some way to tie the two arrays together after > the sort, yes? Or just use the UV in the hash lookup? And what about > floats? The use case is for simple positive integer placeholder index values so I think it would be reasonable to only do numeric sorting for them. (We're not trying to be very general here, just general enough for the few cases where it's worth writing in C to get the speed. Anyone needing finer control can write their own in perl.) > > That would avoid the frequent strtod() calls during sorting (and the > > messing with errno) and avoid problems of accidentally doing a numeric > > sort when not all keys are numeric. > > I think the only time errno is set is for an under/overflow condition?... I > could probably be ignored here. Perhaps, but the extra couple of lines don't cost much and are "the right thing to do", just in case. > >> + look_num = sv_2mortal(newSVpv(keys[0],0)); > >> + if (looks_like_number(look_num)) > >> + sort = _cmp_number; > >> + else > >> + sort = _cmp_str; > > > > I'd set sort_order = (looks_like_number(look_num)) ? 1 : 0; > > here and change the 'else if' below to an 'if'. Simpler logic. > > Okay. My plan was to change the 'else if's into a array of function > pointers that that the sort_order indexed into, so to add a new sort order > all you would do add an entry to the array of function pointers. Probably overkill (YAGNI). > >> +SV * > >> +_concat_hash_sorted (hash, kv_separator, pair_separator, > >> value_format,sort_type) > > >> + hv_len = hv_iterinit(hash); > >> + /* total_len += Separators + quotes + term null */ > >> + total_len += kv_sep_len*hv_len + pair_sep_len*hv_len+2*hv_len+1; > >> + joined = malloc(total_len*sizeof(char)); > > > > I think it would be better to keep appending to an sv rather than try to > > pre-guess the size. That would avoid the need for the malloc/realloc/free. > > Well it would avoid my having to do it; Perl still does.... > > > Probably slightly more expensive, but not by much if you pre-grow the sv. SvGROW(sv, estimated_len) > But it also eliminates the copy of joined to an SV, so it might be a wash? Perhaps. The shorter cleaner code would be nicer for maitenance as well. Thanks again Rudolf. Tim.
