On 2015-01-25 14:59-0500 Jim Dishaw wrote:

>
> On Jan 25, 2015, at 1:26 PM, "Alan W. Irwin" <ir...@beluga.phys.uvic.ca> 
> wrote:
>
>> On 2015-01-24 22:34-0500 Jim Dishaw wrote:
>>
>>> Also, I would like to rewrite plAlloc2dGrid to use only one alloc
>> because the multiple alloc algorithm that is currently used is
>> unnecessary, slower, and on some OSes results in a fragmented heap. Also, by 
>> using one alloc call, plFree2dGrid needs only one free.
>>
>> I think not.  I am pretty sure from code comments I have read in the
>> past, the point of the two-dimensional mallocs (and frees) is to make
>> dealing with dynamical two-dimensional arrays exactly the same as
>> static two-dimensional arrays.  That is a big benefit of the current
>> code I would not want to give up.
>>
> The behavior of the two dimensional arrays would be exactly the same.  
> Instead of doing a malloc for the row pointers and then a series of mallocs 
> for each row, one malloc call is made for all the required space.  I use this 
> technique all the time and here is some example code:
>
> void one_allocate(float ***array, int m, int n)
> {
>  void *mem;
>  float **ptr;
>  int i;
>
>  // Allocate one chunk of memory to store the array data
>  mem = malloc(
>      // Allocate for the row pointer
>      sizeof(float *) * m
>      // Allocate for the rows
>      + sizeof(float) * m * n);
>
>  // Assign top of the memory chunk to
>  ptr = (float **)mem;
>
>  // Advance the memory pointer past the row pointer
>  mem = (void *)((float **)mem + m);
>
>  // Point each row pointer to a chunk of memory
>  // for storing one rows worth of data
>  for(i = 0; i < m; i++) {
>    ptr[i] = (float *)mem;
>    mem = (void *)((float *)mem + n);
>  }
>
>  (*array) = ptr;
> }
>
> I ran a benchmark and the one_allocate approach was almost 17 times faster (I 
> allocated and freed a 20 x 20 array).

Hi Jim:

The above is an obviously better approach for mallocing 2D arrays, and
I am not surprised it is a lot faster.  So my apologies for objecting
before seeing your code.

Please make a separate commit for the change you mentioned before of
splitting off plAlloc2dGrid and related functions to their own file.
Also, please make a separate commit where plAlloc2dGrid is rewritten
as above (with corresponding changes to plFree2dGrid).

If the rest of your plbuf changes are ready, you could just make those
two commits part of a larger patch series you prepare with git
format-patch and attach to your post to this list.  Or you could send
those changes separately as a two-commit patch series (also generated with
git format-patch).  Anyhow, once I see those two commits (on their
own or part of a larger patch series) I will follow up
by testing and pushing those two commits to master, and I will leave
all the rest of your plbuf changes for Phil to evaluate.

>
>
>>> [From a separate post] Does anyone build plplot with BUFFERED_FILE turned on
>>
>> I have looked through the entire source tree with the following
>> command for mentions of BUFFERED_FILE.
>>
>> software@raven> find -type f |grep -v .git |xargs grep -l BUFFERED_FILE
>> ./src/plcore.c
>> ./src/plbuf.c
>> ./include/plstrm.h
>>
>> And if you drop the -l option so you see all the instances, it is
>> clear the BUFFERED_FILE macro is only used and not actually set.
>> Actually there is substantial danger of a name clash here so
>> "BUFFERED_FILE" could be set in some system header somewhere.  So at
>> minimum you should change this to PL_BUFFERED_FILE.  However, I agree
>> with your conclusion that this is just code clutter since it is
>> definitely not set in our current CMake-based build system (in place
>> for almost a decade now) and likely not set in our previous build
>> system either. So I think (unless someone steps forward now to say
>> this is experimental code they meant to get back to working on some
>> day) you should go ahead and completely remove the code (from all 3
>> files above) that would be compiled if BUFFERED_FILE was ever #defined
>> or specified as a compiler option.
>>
> I added the BUFFERED_FILE ifdef blocks to turnoff the original temporary file 
> buffers when I implemented memory buffers.  I did not want to delete the 
> original code in case the memory buffers was a horrible idea or if someone 
> had a low memory machine.  I don't think anyone uses it because, as you 
> pointed out, it is not in the documentation.

So your BUFFERED_FILE ifdef blocks have replaced temporary file
buffers with memory buffers for a long time now, and nobody has had
issues with that change.  So I agree with you that it is time to
remove the temporary file buffer approach completely.

Alan
__________________________
Alan W. Irwin

Astronomical research affiliation with Department of Physics and Astronomy,
University of Victoria (astrowww.phys.uvic.ca).

Programming affiliations with the FreeEOS equation-of-state
implementation for stellar interiors (freeeos.sf.net); the Time
Ephemerides project (timeephem.sf.net); PLplot scientific plotting
software package (plplot.sf.net); the libLASi project
(unifont.org/lasi); the Loads of Linux Links project (loll.sf.net);
and the Linux Brochure Project (lbproject.sf.net).
__________________________

Linux-powered Science
__________________________

------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
Plplot-devel mailing list
Plplot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/plplot-devel

Reply via email to