On Wed, Jul 24, 2013 at 10:48 AM, Nicolas Desprès <[email protected] > wrote:
> > > > On Tue, Jul 23, 2013 at 5:59 PM, Stephen Kelly <[email protected]> wrote: > >> Nicolas Desprès wrote: >> >> > Hi Stephen, >> > >> > Did you have any chance to look at the code? I would love to see it >> > integrated in the next release. That being said, there is no pressure. >> > >> >> I'll have a look now. >> > Thanks! > >> >> >> + UpdateOutputToSourceMap(outputs, file); >> >> is missing a 'this->', as per the style. There's a couple of repeats of >> that. >> > Done > >> >> Please rename >> >> typedef std::map<std::string, cmSourceFile*> OutputToInputMap; >> >> to OutputToSourceMap for similarity to the OutputToSource variable. >> > Done > >> >> I haven't tried it out yet, but it looks sane to me. The >> UpdateOutputToSourceMap could probably be faster if outputs is sorted and >> you use lower-bound insertion in a loop over the map or so. If it's fast >> enough already though, probably no need for that :). >> > Currently, it is fast enough. The path to optimize was search not the > insertion. I have not noticed any performance regression in the insertion. > I liked your idea of inserting in at the end of list/vector and then to > sort it once the configuration is done but I am afraid this require more > knowledge of cmake code base than I have and the patch will be harder to > write. > > That's said we can optimize further as I mentioned in my comment ( > https://github.com/nicolasdespres/CMake/commit/59c871da8b00554812e93ba9c6e47d864424efb0#L0R2023). > Do you have an opinion about it? > > That was not true! It was fastest because it was not doing the right thing. I tried to patch it properly and the benchmark are the same whether we use the default comparison functor or mine. So I think you can merge it like that. I have pushed a new version without the comment. Thaks, -- Nicolas Desprès
-- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
