code review 12720043: runtime: implement SysUnused on windows

runtime: implement SysUnused on windows
Fixes issue 5584.

[dvyukov, 2013-08-12 08:29:20]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://dvyukov%40google.com@code.google.com/p/go/

[chai2010, 2013-08-12 10:07:34]

https://codereview.appspot.com/12720043/diff/10001/src/pkg/runtime/mem_plan9.c
File src/pkg/runtime/mem_plan9.c (right):

https://codereview.appspot.com/12720043/diff/10001/src/pkg/runtime/mem_plan9....
src/pkg/runtime/mem_plan9.c:61: USED(v, nbytes);
typo ?

	USED(v);
	USED(n);

[dvyukov, 2013-08-12 10:36:03]

https://codereview.appspot.com/12720043/diff/10001/src/pkg/runtime/mem_plan9.c
File src/pkg/runtime/mem_plan9.c (right):

https://codereview.appspot.com/12720043/diff/10001/src/pkg/runtime/mem_plan9....
src/pkg/runtime/mem_plan9.c:61: USED(v, nbytes);
On 2013/08/12 10:07:34, chai2010 wrote:
> typo ?
> 
> 	USED(v);
> 	USED(n);

I've done it as it is done few lines above and few lines below in this file.

[brainman, 2013-08-13 01:02:09]

windows part LGTM.

Even better then I expected - you're actually decommitting memory. I don't know
enough about mheap.c, so I didn't check that. Thank you.

Alex

https://codereview.appspot.com/12720043/diff/10001/src/pkg/runtime/mem_windows.c
File src/pkg/runtime/mem_windows.c (right):

https://codereview.appspot.com/12720043/diff/10001/src/pkg/runtime/mem_window...
src/pkg/runtime/mem_windows.c:35: runtime·stdcall(runtime·VirtualFree, 3, v, n,
(uintptr)MEM_DECOMMIT);
Please check for failure, just like SysFree.

https://codereview.appspot.com/12720043/diff/10001/src/pkg/runtime/mem_window...
src/pkg/runtime/mem_windows.c:41: runtime·stdcall(runtime·VirtualAlloc, 4, v, n,
(uintptr)MEM_COMMIT, (uintptr)PAGE_READWRITE);
Please check returned value is equal to v, just like SysMap does.

[dvyukov, 2013-08-13 10:07:58]

https://codereview.appspot.com/12720043/diff/10001/src/pkg/runtime/mem_windows.c
File src/pkg/runtime/mem_windows.c (right):

https://codereview.appspot.com/12720043/diff/10001/src/pkg/runtime/mem_window...
src/pkg/runtime/mem_windows.c:35: runtime·stdcall(runtime·VirtualFree, 3, v, n,
(uintptr)MEM_DECOMMIT);
On 2013/08/13 01:02:09, brainman wrote:
> Please check for failure, just like SysFree.

Done.

https://codereview.appspot.com/12720043/diff/10001/src/pkg/runtime/mem_window...
src/pkg/runtime/mem_windows.c:41: runtime·stdcall(runtime·VirtualAlloc, 4, v, n,
(uintptr)MEM_COMMIT, (uintptr)PAGE_READWRITE);
On 2013/08/13 01:02:09, brainman wrote:
> Please check returned value is equal to v, just like SysMap does.

Done.

[dvyukov, 2013-08-13 10:11:53]

On 2013/08/13 01:02:09, brainman wrote:
> windows part LGTM.
> 
> Even better then I expected - you're actually decommitting memory.

I've tested with MEM_RESET, and it does not actually help -- I started N
processes, each process pages in 1GB of memory and them marks it as MEM_RESET.
It still kills my machine. With MEM_DECOMMIT it works perfectly, even RSS drops
in task manager (not the case with MEM_RESET).

[brainman, 2013-08-13 11:52:28]

On 2013/08/13 10:11:53, dvyukov wrote:
> 
> I've tested with MEM_RESET, and it does not actually help -- I started N
> processes, each process pages in 1GB of memory and them marks it as MEM_RESET.
> It still kills my machine. With MEM_DECOMMIT it works perfectly, even RSS
drops
> in task manager (not the case with MEM_RESET).

Yes, Windows does care about committed memory.

Alex

[dvyukov, 2013-08-14 17:54:15]

*** Submitted as https://code.google.com/p/go/source/detail?r=727c220568e9 ***

runtime: implement SysUnused on windows
Fixes issue 5584.

R=golang-dev, chaishushan, alex.brainman
CC=golang-dev
https://codereview.appspot.com/12720043

[chai2010, 2013-08-15 00:02:12]

https://codereview.appspot.com/12720043/diff/10001/src/pkg/runtime/mem_plan9.c
File src/pkg/runtime/mem_plan9.c (right):

https://codereview.appspot.com/12720043/diff/10001/src/pkg/runtime/mem_plan9....
src/pkg/runtime/mem_plan9.c:61: USED(v, nbytes);
On 2013/08/12 10:36:03, dvyukov wrote:
> On 2013/08/12 10:07:34, chai2010 wrote:
> > typo ?
> > 
> > 	USED(v);
> > 	USED(n);
> 
> I've done it as it is done few lines above and few lines below in this file.

Sorry for this question again.
I can't find the var `nbytes` in this func,
maybe it should change the `n` argument to `nbytes`?

PS:
where is the manual about the `USED` macro?
I can't understand it multi-args behavior.
Thanks.

[brainman, 2013-08-15 00:11:09]

On 2013/08/15 00:02:12, chai2010 wrote:
> Sorry for this question again.
> I can't find the var `nbytes` in this func,
> maybe it should change the `n` argument to `nbytes`?
> 

I don't understand your question. All functions are short here, so 'n' is a fine
variable name.

> where is the manual about the `USED` macro?

I don't know. Did you look in $GOROOT/include? Just look and you will find.

> I can't understand it multi-args behavior.

Never used it in such way. But, if you find the source, you can see it for
yourself.

Alex

[chai2010, 2013-08-15 09:40:58]

On 2013/08/15 00:11:09, brainman wrote:
> On 2013/08/15 00:02:12, chai2010 wrote:
> > Sorry for this question again.
> > I can't find the var `nbytes` in this func,
> > maybe it should change the `n` argument to `nbytes`?
> > 
> 
> I don't understand your question. All functions are short here, so 'n' is a
fine
> variable name.
> 
> > where is the manual about the `USED` macro?
> 
> I don't know. Did you look in $GOROOT/include? Just look and you will find.
> 
> > I can't understand it multi-args behavior.
> 
> Never used it in such way. But, if you find the source, you can see it for
> yourself.
> 
> Alex

sorry for my poor english and poor knowledge.

I have 2 questions:

1. where is `nbytes` defined [3]?

	runtime·SysUsed(void *v, uintptr n)
	{
		USED(v, nbytes); // what is `nbytes` ?
	}

src/pkg/runtime/mem_plan9.c line61 [3]:

2. Does `USED` macro can support multi-arguments?

I look the `USED` defined code in the $GOROOT/include/libc.h:

	/* compiler directives on plan 9 */
	#define SET(x)  ((x)=0)
	#define USED(x) if(x){}else{}
	#ifdef __GNUC__
	#       if __GNUC__ >= 3
	#               undef USED
	#               define USED(x) ((void)(x))
	#       endif
	#endif

`USED` only one argument, but mem_plan9.c used `USED` with two arguments[2,3,4]:

	USED(v, nbytes);

after C preprocessor:

	USED(v, nbytes) -> if(v, nbytes){}else{}
	or
	USED(v, nbytes) -> ((void)(v, nbytes))

Does comma expression (i don't know)?

Thanks!

Reference:

[1]
https://code.google.com/p/go/source/browse/include/libc.h#369

[2]
https://code.google.com/p/go/source/browse/src/pkg/runtime/mem_plan9.c#55

[3]
https://code.google.com/p/go/source/browse/src/pkg/runtime/mem_plan9.c#61

[4]
https://code.google.com/p/go/source/browse/src/pkg/runtime/mem_plan9.c#67

[dvyukov, 2013-08-15 09:49:00]

On 2013/08/15 00:02:12, chai2010 wrote:
> https://codereview.appspot.com/12720043/diff/10001/src/pkg/runtime/mem_plan9.c
> File src/pkg/runtime/mem_plan9.c (right):
> 
>
https://codereview.appspot.com/12720043/diff/10001/src/pkg/runtime/mem_plan9....
> src/pkg/runtime/mem_plan9.c:61: USED(v, nbytes);
> On 2013/08/12 10:36:03, dvyukov wrote:
> > On 2013/08/12 10:07:34, chai2010 wrote:
> > > typo ?
> > > 
> > > 	USED(v);
> > > 	USED(n);
> > 
> > I've done it as it is done few lines above and few lines below in this file.
> 
> Sorry for this question again.
> I can't find the var `nbytes` in this func,
> maybe it should change the `n` argument to `nbytes`?


You're right!
Sent
https://codereview.appspot.com/12986043/diff/3001/src/pkg/runtime/mem_plan9.c

[brainman, 2013-08-16 00:08:08]

On 2013/08/15 09:40:58, chai2010 wrote:
> 
> sorry for my poor english and poor knowledge.
> 

That is two of us :-)

> 
> 2. Does `USED` macro can support multi-arguments?
> 
> I look ...
> 
> 	USED(v, nbytes) -> if(v, nbytes){}else{}
> 	or
> 	USED(v, nbytes) -> ((void)(v, nbytes))
> 

Both looks fine to me. So not more magic.

Alex

[ality, 2013-08-16 01:10:31]

>> 2. Does `USED` macro can support multi-arguments?

The runtime is not compiled using the host compiler
so the macro defined in src/lib9 is not applicable.

In the Plan 9 C compilers, USED and SET are reserved
keywords that act like functions and expect zero or
more arguments (which must be either variable names
or & expressions).

  Anthony