Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memory allocations in sparse_graph.pyx #21720

Closed
jdemeyer opened this issue Oct 18, 2016 · 20 comments
Closed

Fix memory allocations in sparse_graph.pyx #21720

jdemeyer opened this issue Oct 18, 2016 · 20 comments

Comments

@jdemeyer
Copy link

  1. Use checking allocation functions from cysignals to avoid a crash when running out of memory.

  2. A malloc(n) followed by a memset(..., 0, n) should be a calloc().

CC: @jm58660

Component: cython

Author: Jeroen Demeyer

Branch/Commit: 5d1b42e

Reviewer: Marc Mezzarobba

Issue created by migration from https://trac.sagemath.org/ticket/21720

@jdemeyer jdemeyer added this to the sage-7.5 milestone Oct 18, 2016
@jdemeyer

This comment has been minimized.

@jdemeyer

This comment has been minimized.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

@jdemeyer
Copy link
Author

Commit: 5d1b42e

@jdemeyer
Copy link
Author

New commits:

5d1b42eFix memory allocations

@jdemeyer jdemeyer changed the title Fix some memory allocations in sparse_graph.pyx Fix memory allocations in sparse_graph.pyx Oct 18, 2016
@jdemeyer

This comment has been minimized.

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Oct 18, 2016

comment:8

I don't know enough cython to review this, but at least it seems to work. With /proc/sys/vm/overcommit_memory as 0 or 1 it ends with MemoryError: failed to allocate 18446744072098938880 * 8 bytes, with 2 the error message is MemoryError: failed to allocate 1342177280 * 8 bytes.

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Oct 18, 2016

comment:9

Btw, is there same problem for example in src/sage/combinat/degree_sequences.pyx

    sig_on()
    seq = <unsigned char *> sig_malloc((n+1)*sizeof(unsigned char))
    memset(seq,0,(n+1)*sizeof(unsigned char))
    sig_off()

?

@jdemeyer
Copy link
Author

comment:10

I can't fix all problems in Sage...

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Oct 18, 2016

comment:11

Replying to @jdemeyer:

I can't fix all problems in Sage...

I know... But will it help someone if I open a ticket and search other uses of unnecessay memset?

@jdemeyer
Copy link
Author

comment:12

Replying to @jm58660:

Replying to @jdemeyer:

I can't fix all problems in Sage...

I know... But will it help someone if I open a ticket and search other uses of unnecessay memset?

To be honest... unless you plan to fix it yourself, there is probably not much point.

@jdemeyer
Copy link
Author

comment:13

Replying to @jm58660:

With /proc/sys/vm/overcommit_memory as 0 or 1 it ends with MemoryError: failed to allocate 18446744072098938880 * 8 bytes

What is "it" in the sentence above?

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Oct 18, 2016

comment:14

Replying to @jdemeyer:

Replying to @jm58660:

With /proc/sys/vm/overcommit_memory as 0 or 1 it ends with MemoryError: failed to allocate 18446744072098938880 * 8 bytes

What is "it" in the sentence above?

Error message.

@jdemeyer
Copy link
Author

comment:15

Replying to @jm58660:

Replying to @jdemeyer:

Replying to @jm58660:

With /proc/sys/vm/overcommit_memory as 0 or 1 it ends with MemoryError: failed to allocate 18446744072098938880 * 8 bytes

What is "it" in the sentence above?

Error message.

The error message when doing what?

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Oct 18, 2016

comment:16

Uh, sorry. I tested Graph(10^10), but wrote that only in sage-devel, not here.

@jdemeyer
Copy link
Author

comment:17

Replying to @jdemeyer:

Replying to @jm58660:

With /proc/sys/vm/overcommit_memory as 0 or 1 it ends with MemoryError: failed to allocate 18446744072098938880 * 8 bytes

By the way, this number shows that some integer overflow is happening also (a different issue from this ticket).

The number 18446744072098938880 equals <size_t><int>(10 << 28) where the correct value would be just <size_t>(10 << 28) = 2684354560.

@jdemeyer
Copy link
Author

jdemeyer commented Nov 2, 2016

comment:18

Please review...

@mezzarobba
Copy link
Member

Reviewer: Marc Mezzarobba

@vbraun
Copy link
Member

vbraun commented Nov 7, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants