[gdal-dev] Starting a discussion on style and coding guidelines

Fri Jun 3 06:54:24 PDT 2016

I would make some of assertions (some just restating what Even wrote)

* The cost to heap allocation is a fair question (but I would suggest we
defer the work until required)

* Any work done on performance critical subroutines should be done
carefully.

* It would help to have a standard comment that people can add to
empirically detected performance critical sections so that we reduce the
chance of performance regressions

* It would be nice to have a very simple performance check system, but that
seems like a separate discussion that will open the whole benchmarking
can-o-worms.

* It would be great to have a set of instructions on how to instrument GDAL
to collect metrics of runs and detect hot spots with open source tools.
It's been a long time since I've done anything like that with generally
available tools.  e.g. https://sourceware.org/binutils/docs/gprof/  And I
need to look more at the gcov output.  I see that mapserver has a start
https://trac.osgeo.org/mapserver/wiki/PerformanceTesting

* The gdal wiki needs to have more of
http://erouault.blogspot.com/2016/01/software-quality-improvements-in-gdal.html
blended in

* https://en.wikiquote.org/wiki/Donald_Knuth "premature optimization is the
root of all evil (or at least most of it) in programming" :)

* I would suggest that with any guideline there will almost always be the
occasional location where that guideline does not make sense.  In that
case, I would expect that a comment would go along with the exception
explaining why it is important.

* The cost of heap versus stack is not well done in a standalone test (but
there is value in standalone examples).  Pressures on caches, TLBs, etc are
difficult to replicate in simple examples.  Moving a large allocate to the
heap helps keep the rest of the stack fast.  Think about the pages required
for the stack with multiple cores and lots of threads.
http://unix.stackexchange.com/questions/128213/how-is-page-size-determined-in-virtual-address-space

* There are also places where heap allocation is just flat out banned after
a process is initialized.  e.g. daemons like gpsd or long running realtime
systems (your car's engine controllers, spacecraft control systems, etc.).
I would argue that it is never going to be a good idea to use GDAL inside
that kind of system / process

In terms of GDAL, I haven't seen any places where heap allocation of large
objects would like be an important fraction of runtime.  I'm sure they are
there, but I would bet they are rare.

On Thu, Jun 2, 2016 at 3:29 AM, Even Rouault <even.rouault at spatialys.com>
wrote:

> Le jeudi 02 juin 2016 00:46:06, Kurt Schwehr a écrit :
> >
> https://docs.google.com/document/d/1O1B7LY13L532kXcYcB2EdO65m5LOCsqaqn5R9iJ
> > fSPU/pub
> >
> > The optimized stack .o is 1248 bytes and the on the heap vector is 1600
> > bytes with gcc 4.8.  The cost of either is pretty small.  So, if there
> are
> > 100 of these in gdal, we are talking about 30-60K of extra object file
> size
> > for all the cases in GDAL.
>
> Code size is one thing, but I'd be curious to know about the speed impacts
> due
> to the heap allocation. Like time several million iterations of each way
> (and
> make sure to do something non trivial with the arrray so that the compiler
> doesn't optimize the code away). If there's no significant difference,
> then fine.
> Otherwise we'd have to be careful when the replacements are done in a
> performance sensitive routine (my feeling is that there are not so many
> such
> places)
>
> >
> > For stack usage, start thinking about tons of threads spread out over
> lots
> > of cores and think about how systems are built...  Google Compute Engine
> > has 32 core machines.  If you want to get the most out your machines at
> > scale, you want a small stack.
> >
> > int anVals[256] = {};  Does initialize everything to the default value
> > (zeros), but doesn't solve the stack issue.
> >
> > Making a little class is yet another thing for people to learn when
> vector
> > looks to me to work quite well.
> >
> > On Mon, May 9, 2016 at 1:37 PM, Andrew Bell <andrew.bell.ia at gmail.com>
> >
> > wrote:
> > > On Mon, May 9, 2016 at 2:49 PM, Mateusz Loskot <mateusz at loskot.net>
> wrote:
> > >> Point taken.
> > >>
> > >> Although the proposal looks OK, I'd suggest to check what
> > >> assembler code generates your favourite C++ toolkit,
> > >> or at least measured times for
> > >>
> > >> int anVals[256];
> > >> memset(anVals, 0, 256*sizeof(int));
> > >
> > > Are "we" doing memset anymore in these cases?
> > >
> > > int anVals[256] = {};
> > >
> > > seems preferable
> > >
> > >> vs
> > >>
> > >> std::vector<int> oVals(256, 0);
> > >>
> > >> and compare with:
> > >>
> > >> std::vector<char> oVals(256, 0);
> > >
> > > Think vector is a bad solution for something that's fixed.  Just write
> > > something.  But I already suggested that and wrote something... :)
> > >
> > > Do you know why they are wedded to a 16K stack?
> > >
> > > --
> > > Andrew Bell
> > > andrew.bell.ia at gmail.com
> > >
> > > _______________________________________________
> > > gdal-dev mailing list
> > > gdal-dev at lists.osgeo.org
> > > http://lists.osgeo.org/mailman/listinfo/gdal-dev
>
> --
> Spatialys - Geospatial professional services
> http://www.spatialys.com
>

-- 
--
http://schwehr.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/gdal-dev/attachments/20160603/b746aad1/attachment.html>