Fixed in r4439

I'll take care of it, I just want Gabi to finish with the command line
parameters first (I saw she completely changed the main() function).

Erez.

On Mon, Jul 19, 2010 at 6:52 PM, Malte Helmert <
downward.issues@googlemail.com> wrote:

>
> Malte Helmert <helmert@informatik.uni-freiburg.de> added the comment:
>
> The system call added in r4414 broke the release build:
>
> # make release
> g++ -m32 -Wall -W -Wno-sign-compare -Wno-deprecated -ansi -pedantic -Werror
> -g
> -O3 -DNDEBUG -fomit-frame-pointer -static -static-libgcc -c planner.cc -o
> obj/planner.release.o
> cc1plus: warnings being treated as errors
> planner.cc: In function ‘int main(int, const char**)’:
> planner.cc:110: error: ignoring return value of ‘int system(const char*)’,
> declared with attribute warn_unused_result
> make: *** [obj/planner.release.o] Error 1
>
>
> For now, I've applied a temporary fix (r4435), but a better solution is not
> to
> use two external processes (bash + grep) as outlined below.
>
> (BTW, feel free to assign to me if you'd prefer to work on something else.)
>
> _______________________________________________________________________
> Fast Downward issue tracker <downward.issues@gmail.com>
> <http://alfons.informatik.uni-freiburg.de:8088/downward-issues/issue63>
> _______________________________________________________________________
>

-- 

--------------------------------------------------------------
"Adventure is just bad planning."
    Roald Amundsen
    Norwegian Arctic & Antarctic explorer
    (1872 - 1928)
--------------------------------------------------------------

The system call added in r4414 broke the release build:

# make release
g++ -m32 -Wall -W -Wno-sign-compare -Wno-deprecated -ansi -pedantic -Werror -g
-O3 -DNDEBUG -fomit-frame-pointer -static -static-libgcc -c planner.cc -o
obj/planner.release.o
cc1plus: warnings being treated as errors
planner.cc: In function ‘int main(int, const char**)’:
planner.cc:110: error: ignoring return value of ‘int system(const char*)’,
declared with attribute warn_unused_result
make: *** [obj/planner.release.o] Error 1


For now, I've applied a temporary fix (r4435), but a better solution is not to
use two external processes (bash + grep) as outlined below.

(BTW, feel free to assign to me if you'd prefer to work on something else.)

re signals: it'd definitely be good to have this info always -- not sure if you
can catch any signals that occur during timeout due to ulimit -t, but that would
definitely be useful. Also, I think it'd be useful to print some memory usage
statistics whenever we currently give runtime info, e.g. upon search progress,
after h computation, etc.

I guess the best way to proceed is to add some kind of global "give statistics"
function that gives time and memory info and is called wherever we currently
give time info.

Sounds good!

I know I'm a terrible perfectionist, but it's not good style to run two external
processes (bash + grep) for this; it'd be better to open the file ourselves as
an ifstream.

(As a small thing, the rest of the code uses istringstreams instead of sprintf
et al., and I think it'd be good to leave it that way so that we don't have to
worry about buffer overflows, although that's not an issue here.)

So something like (untested, probably not actual code):

istringstream filename;
filename << "/proc/" << getpid() << "/status";

ifstream infile(filename.cstr());

// parse stuff...

These 3 lines at the end do the trick:

char filename[256];
sprintf(filename, "grep VmPeak /proc/%d/status", (int)getpid());
system(filename);

If you want me to print this out also when the planner crashes, let me know and 
I'll catch these signals.

Checked the licence stuff -- the code is by Waschdl; we could use it. (Although,
as I said, we probably should be looking for something that measures peak memory
instead.)

The licence comment was about whether we'd be able to use that code.
I've contacted the author.

In any case, I don't think we want to use it -- you're right, it only reports
current usage, not peak usage, and this is not very good, especially once we add
proper cleanup code to our search algorithms. ;-)

There's a bunch of cases where we have significant memory bottlenecks in
"intermediate" parts of the code that would be important to profile, e.g. the
h^m landmark computation and preparation of merge-and-shrink abstractions.

I googled around a bit and found a (Linux-only, but I don't care) solution that
seems to work well: reading VmPeak from /proc/$(pid)/status. Apparently this is
the *peak* memory usage throughout the lifetime of the process, exactly what we
want. If we read that out just before terminating the process, we should be fine.

Some links:
    http://www.kernel.org/doc/man-pages/online/pages/man5/proc.5.html
    http://serverfault.com/questions/11550/peak-memory-usage-of-a-process

I've added the MemStat file to the planner, and it now reports some data (not 
checked in yet).

I'm not sure it's peak virtual memory size, I think it's the size of the virtual 
memory when the planner terminates (or crashes), but I think that's usually also 
the peak (unless we use lots and lots of memory before search and then free it).

I'm not sure about the comment about the license - Malte, could you provide some  
guidance about this?

See issue51.

Note before adding this to the planner: check with Waschdl that the licence of
this code is fine for inclusion.

Integrate some code to report peak memory usage before quitting.
(Attached to this is the code Waschdl uses for this purpose.)