> Date: Thu, 8 Mar 2012 21:37:50 +0100
> From: wmgcnfg@web.de
> To: mkgmap-dev@lists.mkgmap.org.uk
> Subject: Re: [mkgmap-dev] Merging back the performance branch
>
> I've commited the other changes also.
> I have removed the System.out calls from the LocationHook with a
> separate logger. There might be some questions why a given street or POI
> is not assigned to a special city. To answer such questions it is
> necessary to be enable logging in an mkgmap build.

Okay, I'll look at this tomorrow.


>
> There are also similar debug System.outs in the BoundaryQuadTree.
> Although I think they should be replaced with log statements (it's the
> debugging facility of mkgmap) I have left them there because I think the
> output is mostly interesting for you.

see above

>
> Did I miss any patch? I think I have commited all but there were so
> many... :-)

yes, there was one more :
styledConv_v1.patch was posted 2012-03-03 17:35

Today I have found a good solution to reduce the number of
tag evaluations by detecting common subexpresesions in rules
and caching the results.
Maybe I can finish that tomorrow.
On my machine, mkgmap is now often waiting for the I/O :-)

Ciao,
Gerd


>
> WanMil
>
> > Hi Gerd,
> >
> > regards the findbugs fixes:
> > I have commited the StyleTester change. And I have changed the
> > LocationHook synchronization to use a separate static lock object. Using
> > the boundaryDirName could have the same problem like using BOUNDS_OPTION.
> >
> > I am unsure about the other findbugs changes (GmapsuppBuilder and
> > TypCompiler).
> > They look good to me but Steve should know better if they are a good fix
> > or if they remove a required side effect?!?
> >
> > WanMil
> >
> >> Hi WanMil,
> >>
> >> attached are two patches:
> >> 1) findbugs found several problems in the existing sources.
> >> findbugs.patch contains my suggested solution for three of them,
> >> please review
> >>
> >> 2) performance_cleanup_v1.patch contains
> >> - updated javadoc
> >> - removed dead or commented code
> >> - private constructor for stand-alone BoundaryPreparer to get
> >> rid of new parameters like "preparer-out-dir"
> >> - System.out.println -> log.*
> >> - moved readArea() to readStreamRawFormat to get closer to the
> >> trunk version
> >> - LocationHook: findbugs doesn't like
> >> synchronized (BOUNDS_OPTION) {...}
> >> because BOUNDS_OPTION is an interned String. The
> >> patch changes that to
> >> synchronized (boundaryDirName){...}
> >> I am not sure if that is better?
> >>
> >> Gerd
> >>
> >>
> >> > Date: Sun, 4 Mar 2012 17:18:41 +0100
> >> > From: wmgcnfg@web.de
> >> > To: mkgmap-dev@lists.mkgmap.org.uk
> >> > Subject: Re: [mkgmap-dev] Merging back the performance branch
> >> >
> >> > > Hi WanMil,
> >> > >
> >> > >
> >> > > > Date: Sun, 4 Mar 2012 16:28:12 +0100
> >> > > > From: wmgcnfg@web.de
> >> > > > To: mkgmap-dev@lists.mkgmap.org.uk
> >> > > > Subject: [mkgmap-dev] Merging back the performance branch
> >> > > >
> >> > > > Hi Gerd,
> >> > > >
> >> > > > in your opinion what is the current status of the performance branch?
> >> > > > There seem to be no more fundamental changes so I would like to
> >> focus on
> >> > > > merging it back to the trunk. Additional improvements can then be
> >> > > > applied to the trunk.
> >> > >
> >> > > well, r2234 doesn't contain all patches that I've posted. Do you
> >> plan to
> >> > > add them?
> >> >
> >> > Of course, but I want to have a look on the patches before commiting
> >> them.
> >> >
> >> > >
> >> > > My latest version contains another big change regarding Tags and
> >> > > StyledConverter.
> >> > > I'd like to change the Rule evaluation in a way that it will allow to
> >> > > skip many rules
> >> > > that are now partly evaluated.
> >> > > Up to now my changes are saving maybe 5% - 10 %, but I hope to see
> >> more.
> >> > > I don't think that this will be done soon.
> >> >
> >> > We can merge back the performance branch and continue using it for the
> >> > style changes.
> >> >
> >> > >
> >> > > I have an idea how splitArea() could be changed to be much faster, but
> >> > > no algorithm
> >> > > yet. I believe that we don't need any Area.intersect() calculations for
> >> > > that, we
> >> > > just have to split a few lines.
> >> >
> >> > That's great.
> >> > Maybe you can invest your time better in other points of mkgmap with a
> >> > greater improvement. Performance improvements are great but there are
> >> > some functional problems unsolved, e.g. the Subdivision creation in
> >> > mkgmap has some problems which need to be addressed.
> >> >
> >> > >
> >> > > >
> >> > > > The performance improvements are great. Before merging back I
> >> would like
> >> > > > to invite people (and give them the chance) to test it carefully. For
> >> > > > this I will upload the complete world bounds so that anyone can
> >> > > participate.
> >> > >
> >> > > good. I can't believe that so many changes do not contain any bugs:-)
> >> > >
> >> > > >
> >> > > > From my point of view the following things must be done before
> >> merging
> >> > > > back:
> >> > > > * Remove several System.out printouts. Please use the mkgmap internal
> >> > > > logging system.
> >> > > > * One note to the logging system: Instead of using
> >> > > > log.info("There were "+n+" results")
> >> > > > better use
> >> > > > log.info("There were", n, "results")
> >> > > > This avoids string creation in case the log message is not logged.
> >> > >
> >> > > okay, I will change that.
> >> > >
> >> > > > * Please check the code with FindBugs or something similar
> >> > >
> >> > > okay, I wasn't aware that this tool is for free :-)
> >> > >
> >> > > > * Is the source code well documented?
> >> > >
> >> > > hard to say. I tend to comment only those lines that implement
> >> > > a tricky solution. Everything else should be self-documenting.
> >> > >
> >> > > > * Is the licence header added to all classes?
> >> > > What would the right one?
> >> >
> >> > I don't know if there is a 'correct' one but I am using the following:
> >> > /*
> >> > * Copyright (C) 2006, 2012.
> >> > *
> >> > * This program is free software; you can redistribute it and/or modify
> >> > * it under the terms of the GNU General Public License version 3 or
> >> > * version 2 as published by the Free Software Foundation.
> >> > *
> >> > * This program is distributed in the hope that it will be useful, but
> >> > * WITHOUT ANY WARRANTY; without even the implied warranty of
> >> > * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> >> > * General Public License for more details.
> >> > */
> >> >
> >> > >
> >> > > > * The BoundaryPreparer uses options like "preparer-out-dir". Are
> >> these
> >> > > > options documented? The name should be more specific (e.g.
> >> > > bounds-out-dir).
> >> > > okay
> >> > >
> >> > > Gerd
> >> > >
> >> > >
> >> > > >
> >> > > > WanMil
> >> > > > _______________________________________________
> >> > > > mkgmap-dev mailing list
> >> > > > mkgmap-dev@lists.mkgmap.org.uk
> >> > > > http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> >> > >
> >> > >
> >> > > _______________________________________________
> >> > > mkgmap-dev mailing list
> >> > > mkgmap-dev@lists.mkgmap.org.uk
> >> > > http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> >> >
> >> > _______________________________________________
> >> > mkgmap-dev mailing list
> >> > mkgmap-dev@lists.mkgmap.org.uk
> >> > http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> >>
> >>
> >> _______________________________________________
> >> mkgmap-dev mailing list
> >> mkgmap-dev@lists.mkgmap.org.uk
> >> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> >
> > _______________________________________________
> > mkgmap-dev mailing list
> > mkgmap-dev@lists.mkgmap.org.uk
> > http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
>
> _______________________________________________
> mkgmap-dev mailing list
> mkgmap-dev@lists.mkgmap.org.uk
> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev