cms-dev / cms

Contest Management System
http://cms-dev.github.io/
GNU Affero General Public License v3.0
877 stars 357 forks source link

Fix the memory measurement in cms #757

Closed dobito6 closed 6 years ago

dobito6 commented 7 years ago

I have recently noticed strange behaviour of CMS while executing two identical submissions. Here is the code which I submitted. I estimated that the used memory should be around 762 MiB. I would prefer to show you what I mean.

screenshot 1 screenshot 2

The only diffetence between the submissions is the memory limit which in the first case is set to be 16 MiB while in the second case is 1024 MiB. The time limit is 3 seconds. In the first case I expected to get a message like "Execution killed with signal %d" but it did not happen.

I suppose that after the submission uses all allowed RAM it takes more memory from the swap and because of that solution exceeds the time limit. Actually I tried variety of different ways to get memory limit (execution killed with signal %d) but I did not manage to do this unless I disable the swap.

In my opinion disabling the swap memory (with sudo swapoff -a) is too inconvenient every time when cms is working. Furthermore I think that submissions should be killed immidiately after alllocation of too much memory - something that does not happen after v1.2.0.

I hope you will find a way to fix the problem. Sorry if I have made a mistake somewhere.

stefano-maggiolo commented 7 years ago

Hi! Most probably you haven't disabled the swap space. isolate does not limit swap, just physical memory. Therefore if you have swap space, your program won't be killed, it will just take a long time due to all the swapping.

Run sudo swapoff -a (if your system is not too loaded, otherwise you're going to have trouble) and retry :)

dobito6 commented 7 years ago

Even if I do this, with a declaration of too large array in the program it works fine though it should not be like that. For example in this code on the fifth line there is an array which takes around 924 MiB. This is a real task given on our national olympiad in informatics and on CMS 1.3 or later it gets full score while on CMS 1.2 does not get any points, which is the right according to the final results.

akmohtashami commented 7 years ago

As of c598dbd3c40c617e48507891133569939272ee03, CMS uses -cg-mem instead of -mem. I am not familiar with how isolate works but considering that the -mem option hasn't been deprecated shouldn't we use both of these options instead of replacing one with another? Additionally, considering that CMS allows disabling usage of cgroups, I think -cg-mem shouldn't be used when cgroup is disabled.

Also, do we know why isolate is failing to limit memory and swap together? It seems there is a line explicitly limiting memory plus swap usage in the isolate source code (that is, when cg-mem is specified).

@dobitoto6 Are there testcases which make the program use all the defined cells of the array? Also, could you share what CMS reports as memory usage of the submission?

dobito6 commented 7 years ago

No, there aren't such testcases that require using of the whole array. As far as I can remember the max used memory was approximately 250 MiB. However, I think that CMS should measure all allocated memory instead of the only part that is really used.

andreyv commented 7 years ago

This is behavior is specific to Linux, not CMS. You can read about it here: https://www.kernel.org/doc/Documentation/vm/overcommit-accounting You can change it, but I would not recommend this.

stefano-maggiolo commented 7 years ago

IMHO it's similar to unused variables optimized away by the compiler, so completely fine to pass.

@gollux opinions on using both --cg-mem and --mem? IIUC, these are fairly equivalent for single programs, but in case of multiple processes the former compares with the total memory used, the latter with each process separately.

gollux commented 7 years ago

@gollux opinions on using both --cg-mem and --mem?

I do not have a strong opinion on that. Ignoring memory not allocated by the program (whatever allocation means -- there are multiple possible meanings) looks fine to me. Stefano's analogy with variables optimized away by the compiler makes perfect sense.

Anyway, combining --cg-mem and --mem could save some confusion, so it could be a good idea.

gollux commented 6 years ago

Generally, I do not think that having swap enabled on CMS workers is a good idea. I will add it to the list of caveats in isolate's documentation.

stefano-maggiolo commented 6 years ago

It's already in isolate-check-environment

gollux commented 6 years ago

Yes, I know, but I wanted to put it in the documentation, too.