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

Improve memory allocation performance #1343

Merged
merged 1 commit into from
Jul 27, 2018
Merged

Conversation

okuta
Copy link
Member

@okuta okuta commented Jun 10, 2018

rel #1340.

@okuta okuta added the cat:enhancement Improvements to existing features label Jun 10, 2018
@okuta okuta mentioned this pull request Jun 10, 2018
@okuta okuta added cat:performance Performance in terms of speed or memory consumption to-be-backported Pull-requests to be backported to stable branch labels Jun 17, 2018
@kmaehashi kmaehashi self-assigned this Jun 22, 2018
@okuta okuta force-pushed the improve-memory branch 2 times, most recently from c241f49 to a13db44 Compare June 25, 2018 17:37
@okuta
Copy link
Member Author

okuta commented Jul 17, 2018

jenkins, test this please.

@chainer-ci
Copy link
Member

Jenkins CI test (for commit 6752a44) failed with status FAILURE.
(For contributors, please wait until the reviewer confirms the details of the error.)

@okuta
Copy link
Member Author

okuta commented Jul 17, 2018

jenkins, test this please.

@chainer-ci
Copy link
Member

Jenkins CI test (for commit 6752a44) failed with status FAILURE.
(For contributors, please wait until the reviewer confirms the details of the error.)

@okuta okuta added this to the v5.0.0b4 milestone Jul 23, 2018
@okuta
Copy link
Member Author

okuta commented Jul 23, 2018

jenkins, test this please.

@chainer-ci
Copy link
Member

Jenkins CI test (for commit 6752a44, target branch master) failed with status FAILURE.
(For contributors, please wait until the reviewer confirms the details of the error.)

@okuta
Copy link
Member Author

okuta commented Jul 25, 2018

jenkins, test this please.

@chainer-ci
Copy link
Member

Jenkins CI test (for commit 0d89993, target branch master) succeeded!

@beam2d beam2d self-assigned this Jul 26, 2018
Copy link
Contributor

@beam2d beam2d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except for a minor comment.

@@ -166,10 +167,11 @@ cdef class _Chunk:

def __init__(self, *args):
# For debug
self._init(*args)
mem, offset, size, stream_ptr = args
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you directly write __init__(self, mem, offset, size, stream_ptr)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, it looks this method is only used for debugging. Is it related to the performance improvement?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to avoid compile error because _init is a cdef method.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, ok, understood.

@@ -617,13 +665,12 @@ cdef class SingleDeviceMemoryPool:

cpdef Py_ssize_t _round_size(self, Py_ssize_t size):
"""Rounds up the memory size to fit memory alignment of cudaMalloc."""
unit = self._allocation_unit_size
cdef Py_ssize_t unit = self._allocation_unit_size
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memo: this annotation is needed to let Cython avoid inferring the type of unit as Python int.

@beam2d
Copy link
Contributor

beam2d commented Jul 27, 2018

LGTM

@beam2d beam2d merged commit 1f1fc93 into cupy:master Jul 27, 2018
beam2d added a commit to beam2d/cupy that referenced this pull request Jul 27, 2018
Improve memory allocation performance
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat:enhancement Improvements to existing features cat:performance Performance in terms of speed or memory consumption to-be-backported Pull-requests to be backported to stable branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants