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.
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.
Did I miss any patch? I think I have commited all but there were so
many... :-)
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: [email protected]
>> > To: [email protected]
>> > Subject: Re: [mkgmap-dev] Merging back the performance branch
>> >
>> > > Hi WanMil,
>> > >
>> > >
>> > > > Date: Sun, 4 Mar 2012 16:28:12 +0100
>> > > > From: [email protected]
>> > > > To: [email protected]
>> > > > 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
>> > > > [email protected]
>> > > > http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
>> > >
>> > >
>> > > _______________________________________________
>> > > mkgmap-dev mailing list
>> > > [email protected]
>> > > http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
>> >
>> > _______________________________________________
>> > mkgmap-dev mailing list
>> > [email protected]
>> > http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
>>
>>
>> _______________________________________________
>> mkgmap-dev mailing list
>> [email protected]
>> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
>
> _______________________________________________
> mkgmap-dev mailing list
> [email protected]
> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
_______________________________________________
mkgmap-dev mailing list
[email protected]
http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
