> As you did not provide profiling results before, you implicitly asked for > others to produce these.
I do not want to dig into the politics, but this approach seems wrong to me. I agree verification is important but the author of the contribution (i.e. patch) should provide info on fixes/performance impact etc. By not providing this, contributor *is not* implicitly asking for commiters to do that. If you need a justification for the patch, you should ask contributor to add missing info and if he does not add it, then simply reject the patch. It may seem a bit harsh but there is IMHO no other long-term strategy to this because otherwise you will become cluttered with finishing other people's unfinished job. :) Fiisch 2017-02-14 16:19 GMT+01:00 <win...@genial.ms>: > Appealing to self-authority is a bad choice when clearly showing > lack of knowledge. Please, learn about the Pareto Principle and > Amdahl's Law! > I do not appreciate making strawman arguments from the least > important part of my email where I describe that I have to do > some manual extra steps, as I'm using webmail not Pine on UNIX; > that means not misrepresenting that as not knowing about git am. > I do not appreciate ad hominems about how we supposedly not care > about the code in the repository. > You might profit from learning what to avoid there, as there could > be more: https://yourlogicalfallacyis.com/ > > It would be nice if you also avoided trying to optimize the 90% of code > which are not taking a significant part of the time spent in the > program, as that wastes both your and others time. > > I'd appreciate you following the scientific method. For example, > if you profile and see no measurable improvement, do not try to find > reasons why this test data might be ignored, instead try to improve > things and reconsider your prevailing views. > > Btw., it should be clear someone committing patches needs to > verify that they work and for optimizations theres an even higher > barrier of showing they improve the performance significantly enough > to justify changing the code (which might introduce bugs, make it more > complicated and takes human resources). As you did not provide > profiling results before, you implicitly asked for others to > produce these. > > >> Gesendet: Dienstag, 14. Februar 2017 um 08:09 Uhr >> Von: "Enrico Weigelt, metux IT consult" <enrico.weig...@gr13.net> >> An: win...@genial.ms >> Cc: freecol-developers@lists.sourceforge.net >> Betreff: Re: [Freecol-developers] emails from patches >> >> On 13.02.2017 15:06, win...@genial.ms wrote: >> > Hello, >> > >> > you know how that answer feels? >> >> Like talking to an old kernel hacker, who's used to cope with many >> and large patch queues and therefore tries to get around w/ minimal >> bureaucracy ? :o >> >> Having to create tickets and copy-paste patches on some website >> manually, introduces lots of extra bureaucracy. And if we wanna >> discuss about them, it gets even more bureaucratic - compared with >> simple and efficient tools like email. It doesn't even make applying >> easier - OTOH, git can apply emails (and whole mailboxes) automatically. >> >> > Like you only value your own time, as you don't even invest enough >> > of it to write a tiny justification for each patch. >> >> Actually, I did, where I felt that it's not obvious. >> >> Looking at the repo history, it doesn't seem that the committers there >> seem to care much. >> >> OTOH, many commits there dont seem to be very consistent: eg. >> introducing temporary hacks or even breaking things, which are cleaned >> up again by later commits. Those things I'd really like to prevent, >> by cleaning them up before. >> >> So, can you imagine, how it feels, being critized about things that the >> committers are doing even worse ? Do you think that's how free adult >> people should treat each other ? >> >> By the way: maybe it's just a language problem, but the term >> "justifications" sounds a bit like being tackled by a supervisor or a >> judge. Obviously, we're all free men here, and nobody here has to >> justify himself for another. >> >> > Then it costs others about twice as much time to figure out >> > what to do with such a patch, cause of reverse engineering the reasons >> > you made it from the code, before verifying it actually works. >> >> If you've got any questions, please ask. >> >> > I still saw no numbers from you as to how much cpu time these supposed >> > optimization patches would conserve, and I suspect its because you >> > still did not profile. >> >> Actually, I did a few profilings, but it's hard to get any useful >> numbers, as the jitter is just too high, and it's just not worth >> the effort und patch-by-patch basis. >> >> OTOH, as an seasoned sw-engineer w/ >25yrs experience (who also used >> to build its own computers, aeons ago), I might have some piece of >> understanding, whats going on under the hood and so dont need to >> measure every single piece. >> >> > That means we'd have to take the time to profile >> > and most likely see no significant improvement to justify changing >> > the code. >> >> Why do you need to do that ? Did I ask you to do that ? >> >> For most of my optimization patches it should even be obvious that they >> bring some improvements (whether it's actually noticable in an >> individual case is a different story), for anybody who knows a bit >> about java and jvm. >> >> Lets take a simple but common example: >> >> --- a/src/net/sf/freecol/common/model/Europe.java >> +++ b/src/net/sf/freecol/common/model/Europe.java >> @@ -386,7 +386,8 @@ public class Europe extends UnitLocation >> Player player = getOwner(); >> Market market = player.getMarket(); >> int price = 0; >> - for (AbstractGoods ag : transform(goods, >> AbstractGoods::isPositive)) { >> + for (AbstractGoods ag : goods) { >> + if (!ag.isPositive()) continue; >> GoodsType goodsType = ag.getType(); >> // Refuse to trade in boycotted goods >> if (!player.canTrade(goodsType)) { >> >> The original version does a lot of extra things under the hood, eg: >> * creates a new list (where the filtered elements put it) >> * creates two stream objects (plus several internal ones, eg. >> splititerator, etc) >> * creates a collector object (plus several internal ones) >> * creates a callback object, which is called or for each list element, >> just to call AbstractGoods.isPositive() >> * collect() finally calls (though a huge stack of indirection) through >> the whole stream/splititerator chain, down to the predicate and the >> collector objects, for each list element, until the end of the >> stream is reached. >> >> OTOH, my version skips that all away, just direct invokestatic to >> AbstractGoods::isPositive() (which is easily inlinable, BTW) and >> conditional jump. >> >> Oh, BTW, my version is also using *less* code and is easier to read. >> >> > Then if I would decide on using such a patch I personally would need to >> > copy the text from each email out into a new file and try to apply it. >> >> You could just save the mail as is and apply directly: >> >> https://www.kernel.org/pub/software/scm/git/docs/git-am.html >> >> > You could just post a link to a branch on your SF or Github fork and >> > if its just a whole stack of related commits one posting including >> > a link to a git repository and a few numbers would be better than >> > a stack of emails. >> >> Already did so, nobody seemed to care. >> >> >> >> --mtx >> >> ------------------------------------------------------------------------------ >> Check out the vibrant tech community on one of the world's most >> engaging tech sites, SlashDot.org! http://sdm.link/slashdot >> _______________________________________________ >> Freecol-developers mailing list >> Freecol-developers@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/freecol-developers >> > > ------------------------------------------------------------------------------ > Check out the vibrant tech community on one of the world's most > engaging tech sites, SlashDot.org! http://sdm.link/slashdot > _______________________________________________ > Freecol-developers mailing list > Freecol-developers@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/freecol-developers ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot _______________________________________________ Freecol-developers mailing list Freecol-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freecol-developers
