[issue46606] Large C stack usage of os.getgroups() and os.setgroups()

[STINNER Victor]

STINNER Victor  added the comment:

> NGROUPS_MAX is 65536 and sizeof(gid_t) is 4 on Ubuntu 20.04, so grouplist is 
> 262144bytes.

Oops, that's a lot! Nicely spotted! Yeah, it's perfectly fine to allocate a 
temporary array on the heap memory. There is no need to micro-optimize this 
function.

Maybe the code was written when NGROUPS_MAX was way smaller (64?).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46606] Large C stack usage of os.getgroups() and os.setgroups()

[STINNER Victor]

STINNER Victor  added the comment:


New changeset e02c47528b31f513d5f5d6eb91b8c9714134cea2 by Victor Stinner in 
branch 'main':
bpo-46606: os.getgroups() doesn't overallocate (GH-31569)
https://github.com/python/cpython/commit/e02c47528b31f513d5f5d6eb91b8c9714134cea2


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46606] Large C stack usage of os.getgroups() and os.setgroups()

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +29691
pull_request: https://github.com/python/cpython/pull/31569

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46606] Large C stack usage of os.getgroups() and os.setgroups()

[Inada Naoki]

Inada Naoki  added the comment:


New changeset ad6c7003e38a9f8bdf8d865fb5fa0f3c03690315 by Inada Naoki in branch 
'main':
bpo-46606: Remove redundant +1. (GH-31561)
https://github.com/python/cpython/commit/ad6c7003e38a9f8bdf8d865fb5fa0f3c03690315


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46606] Large C stack usage of os.getgroups() and os.setgroups()

[Inada Naoki]

Change by Inada Naoki :


--
pull_requests: +29684
pull_request: https://github.com/python/cpython/pull/31561

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46606] Large C stack usage of os.getgroups() and os.setgroups()

[STINNER Victor]

STINNER Victor  added the comment:

>  n++; // Avoid malloc(0)
> grouplist = PyMem_New(gid_t, n+1);

FYI PyMem_New(0) is well specified and doesn't return NULL:
https://docs.python.org/dev/c-api/memory.html#c.PyMem_Malloc

--
nosy: +vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46606] Large C stack usage of os.getgroups() and os.setgroups()

[Inada Naoki]

Change by Inada Naoki :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46606] Large C stack usage of os.getgroups() and os.setgroups()

[Inada Naoki]

Inada Naoki  added the comment:


New changeset 74127b89a8224d021fc76f679422b76510844ff9 by Inada Naoki in branch 
'main':
bpo-46606: Reduce stack usage of getgroups and setgroups (GH-31073)
https://github.com/python/cpython/commit/74127b89a8224d021fc76f679422b76510844ff9


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46606] Large C stack usage of os.getgroups() and os.setgroups()

[Dong-hee Na]

Change by Dong-hee Na :


--
nosy: +corona10

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46606] Large C stack usage of os.getgroups() and os.setgroups()

[Inada Naoki]

Change by Inada Naoki :


--
keywords: +patch
pull_requests: +29257
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/31073

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46606] Large C stack usage of os.getgroups() and os.setgroups()

[Inada Naoki]

New submission from Inada Naoki :

I checked stack usage for bpo-46600 and found this two functions use a lot of 
stack.

os_setgroups: 262200 bytes
os_getgroups_impl: 262184 bytes

Both function has local variable like this:

gid_t grouplist[MAX_GROUPS];

MAX_GROUPS is defined as:

```
#ifdef NGROUPS_MAX
#define MAX_GROUPS NGROUPS_MAX
#else
/* defined to be 16 on Solaris7, so this should be a small number */
#define MAX_GROUPS 64
#endif
```

NGROUPS_MAX is 65536 and sizeof(gid_t) is 4 on Ubuntu 20.04, so grouplist is 
262144bytes.

It seems this grouplist is just for avoid allocation:

```
} else if (n <= MAX_GROUPS) {
/* groups will fit in existing array */
alt_grouplist = grouplist;
} else {
alt_grouplist = PyMem_New(gid_t, n);
if (alt_grouplist == NULL) {
return PyErr_NoMemory();
}
```

How about just using `#define MAX_GROUPS 64`?
Or should we remove this grouplist because os.grouplist() is not called so 
frequently?

--
components: Library (Lib)
messages: 412335
nosy: methane
priority: normal
severity: normal
status: open
title: Large C stack usage of os.getgroups() and os.setgroups()
versions: Python 3.11

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com