Hi Brad,
I have force-pushed all the fixes you've asked for:
* I taught cmMakefile copy-ctor
* I re-write the test so that it is part of CMakeOnly sub-test suite. I
also make it more robust to machine load by increasing the number of
targets. This way the performance gap between the logarithmic
On 08/06/2013 09:40 AM, Nicolas Desprès wrote:
I have force-pushed all the fixes you've asked for:
Great, thanks!
* I taught cmMakefile copy-ctor
Good.
* I re-write the test so that it is part of CMakeOnly sub-test suite. I also
make it more robust to machine load by increasing the number
On Tue, Aug 6, 2013 at 4:31 PM, Brad King brad.k...@kitware.com wrote:
On 08/06/2013 09:40 AM, Nicolas Desprès wrote:
I have force-pushed all the fixes you've asked for:
Great, thanks!
* I taught cmMakefile copy-ctor
Good.
* I re-write the test so that it is part of CMakeOnly
On 08/06/2013 12:18 PM, Nicolas Desprès wrote:
The test suite passes on my computer. I have force-pushed again.
Great.
In this hunk:
+ // If the queried path is not absolute we use the backward compatible
+ // version. The search algorithm is linear.
+ if (cname[0] != '/')
Use
On 08/06/2013 12:44 PM, Brad King wrote:
On 08/06/2013 12:18 PM, Nicolas Desprès wrote:
The test suite passes on my computer. I have force-pushed again.
Great.
I had to modify the test like this:
-DEPENDS bottle_neck
+DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/bottle_neck
in order to
On 08/06/2013 01:30 PM, Brad King wrote:
IMO it is not worth the effort to establish this test.
I've applied a patch based on yours to implement the
optimization and merged to 'next' for testing tonight:
http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=2268c41a
-Brad
--
Powered by
On Tue, Aug 6, 2013 at 10:19 PM, Brad King brad.k...@kitware.com wrote:
On 08/06/2013 01:30 PM, Brad King wrote:
IMO it is not worth the effort to establish this test.
I've applied a patch based on yours to implement the
optimization and merged to 'next' for testing tonight:
Hi,
I have just rebased my branch on top of the master and the test suite still
pass.
I have a real world example here with performance. My project's build
system has 7113 commands to run. This is roughly 7 commands for each input
files. There are 1000 inputs files.
Just for the configuration
On 08/05/2013 12:06 PM, Brad King wrote:
Here are a couple comments:
Also I think the cmMakefile copy constructor needs to be taught
about the member you're adding.
-Brad
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
On Mon, Aug 5, 2013 at 6:06 PM, Brad King brad.k...@kitware.com wrote:
On 08/05/2013 11:28 AM, Nicolas Desprès wrote:
I have just rebased my branch on top of the master and the test suite
still pass.
[snip]
I really want to have this patch in the next release
I can't promise to accept it
On Mon, Aug 5, 2013 at 9:50 PM, Brad King brad.k...@kitware.com wrote:
On 08/05/2013 12:06 PM, Brad King wrote:
Here are a couple comments:
Also I think the cmMakefile copy constructor needs to be taught
about the member you're adding.
Yep I do that tomorrow morning. It is late now in
On 08/05/2013 04:37 PM, Nicolas Desprès wrote:
I have pushed a bunch of fixup commit if you want to review them. I will
squash them tomorrow.
Thanks! Don't bother squashing yet. There may be more fixups
needed after testing.
-Brad
--
Powered by www.kitware.com
Visit other Kitware
On 08/05/2013 04:37 PM, Nicolas Desprès wrote:
* Why is special logic in Tests/CMakeLists.txt needed to add
the test case?
Since it is a performance test on configuration phase only, I need a
specific timeout value and to run cmake only
Take a look at the Tests/RunCMake and
Nicolas Desprès wrote:
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.
I
On Mon, Jul 29, 2013 at 12:57 PM, Stephen Kelly steve...@gmail.com wrote:
Nicolas Desprès wrote:
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
Nicolas Desprès wrote:
On Mon, Jul 29, 2013 at 12:57 PM, Stephen Kelly
steve...@gmail.com wrote:
Nicolas Desprès wrote:
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
On Mon, Jul 29, 2013 at 2:09 PM, Stephen Kelly steve...@gmail.com wrote:
Nicolas Desprès wrote:
On Mon, Jul 29, 2013 at 12:57 PM, Stephen Kelly
steve...@gmail.com wrote:
Nicolas Desprès wrote:
It was fastest because it was not doing the right thing. I tried to
patch it properly
Nicolas Desprès wrote:
Also, I don't see why the custom comparison functor is needed at all. I
removed it and sped up the test significantly. Can you explain?
I had some failing tests because the previous algorithm was not a plain
strcpy().
I don't see any strcpy(). Anyway, I think that's
On Mon, Jul 29, 2013 at 2:25 PM, Stephen Kelly steve...@gmail.com wrote:
Nicolas Desprès wrote:
Also, I don't see why the custom comparison functor is needed at all. I
removed it and sped up the test significantly. Can you explain?
I had some failing tests because the previous
On Wed, Jul 24, 2013 at 7:56 PM, Nicolas Desprès
nicolas.desp...@gmail.comwrote:
On Wed, Jul 24, 2013 at 12:18 PM, Stephen Kelly steve...@gmail.comwrote:
Nicolas Desprès wrote:
That's said we can optimize further as I mentioned in my comment (
Nicolas Desprès wrote:
Hi Stephen,
I think I need your insight of CMake code base here. According to my tests
(ie ExportImport and CustomCommand), GetSourceFileWithOutput() is called
with a relative path only from here:
https://github.com/Kitware/CMake/blob/master/Source/cmTarget.cxx#L1939
On Wed, Jul 24, 2013 at 10:48 AM, Nicolas Desprès nicolas.desp...@gmail.com
wrote:
On Tue, Jul 23, 2013 at 5:59 PM, Stephen Kelly steve...@gmail.com 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
On Tue, Jul 23, 2013 at 5:59 PM, Stephen Kelly steve...@gmail.com 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!
Nicolas Desprès wrote:
+UpdateOutputToSourceMap(outputs, file);
is missing a 'this-', as per the style. There's a couple of repeats of
that.
Done
You seem to have missed this one.
There are also other style issues.
* Don't put an else after a return
* Wrap single line blocks in {}
On Wed, Jul 24, 2013 at 11:29 AM, Stephen Kelly steve...@gmail.com wrote:
Nicolas Desprès wrote:
+UpdateOutputToSourceMap(outputs, file);
is missing a 'this-', as per the style. There's a couple of repeats of
that.
Done
You seem to have missed this one.
There are also other
Nicolas Desprès wrote:
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?
Do I understand correctly that the issue is that OutputToSource
On Wed, Jul 24, 2013 at 12:18 PM, Stephen Kelly steve...@gmail.com wrote:
Nicolas Desprès wrote:
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
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.
Thanks,
-Nico
On Tue, Jul 16, 2013 at 1:40 PM, Nicolas Desprès
nicolas.desp...@gmail.comwrote:
Hi Stephen,
I have pushed more commits and
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.
+UpdateOutputToSourceMap(outputs, file);
is missing a 'this-', as per the style.
Hi Stephen,
I have pushed more commits and now the problem is fixed and all the test
suite passes on my machine.
However it could even be faster. See my comment
in cmMakefile::Compare::operator():
On Sat, Jul 13, 2013 at 10:21 AM, Nicolas Desprès nicolas.desp...@gmail.com
wrote:
On Fri, Jul 12, 2013 at 11:35 PM, Stephen Kelly steve...@gmail.comwrote:
Nicolas Desprès wrote:
I have pushed my work so far on my github clone of CMake.
Nicolas Desprès wrote:
I have pushed my work so far on my github clone of CMake.
https://github.com/nicolasdespres/CMake/tree/topic/large-deps-perf
Thanks. The bottleneck seems to be in
cmTargetTraceDependencies::FollowName
as it follows each entry in the LARGE_LIST for each of the 100
On Mon, Jul 15, 2013 at 11:53 AM, Stephen Kelly steve...@gmail.com wrote:
Nicolas Desprès wrote:
I have pushed my work so far on my github clone of CMake.
https://github.com/nicolasdespres/CMake/tree/topic/large-deps-perf
Thanks. The bottleneck seems to be in
On Mon, Jul 15, 2013 at 12:45 PM, Nicolas Desprès nicolas.desp...@gmail.com
wrote:
On Mon, Jul 15, 2013 at 11:53 AM, Stephen Kelly steve...@gmail.comwrote:
Nicolas Desprès wrote:
I have pushed my work so far on my github clone of CMake.
Nicolas Desprès wrote:
Yes that one or cmMakefile::GetSourceFileWithOutput(char const*). I can
send you a gzipped of my callgrind data off-list if you want (it
weights 534K).
If we had a map associating each input to output, we could maybe have
better performance. WDYT?
Perhaps. I can
On Mon, Jul 15, 2013 at 1:51 PM, Stephen Kelly steve...@gmail.com wrote:
Nicolas Desprès wrote:
Yes that one or cmMakefile::GetSourceFileWithOutput(char const*). I can
send you a gzipped of my callgrind data off-list if you want (it
weights 534K).
If we had a map associating each
On Fri, Jul 12, 2013 at 11:35 PM, Stephen Kelly steve...@gmail.com wrote:
Nicolas Desprès wrote:
I have pushed my work so far on my github clone of CMake.
https://github.com/nicolasdespres/CMake/tree/topic/large-deps-perf
Something must have gone wrong with the push :)
Oups, sorry :(.
Nicolas Desprès wrote:
I have pushed my work so far on my github clone of CMake.
https://github.com/nicolasdespres/CMake/tree/topic/large-deps-perf
Something must have gone wrong with the push :)
stephen@hal:~/dev/src/cmake{master}$ git show origin/master | grep commit
commit
38 matches
Mail list logo
