Speed up ElementwiseKernel launch #1318

okuta · 2018-06-03T16:01:20Z

before
python adam.py 7.55s user 0.68s system 99% cpu 8.234 total
python adam.py 7.57s user 0.71s system 99% cpu 8.291 total
python adam.py 7.74s user 0.70s system 99% cpu 8.456 total
python adam.py 7.82s user 0.66s system 99% cpu 8.482 total

after
python adam.py 5.04s user 0.67s system 99% cpu 5.711 total
python adam.py 4.76s user 0.68s system 99% cpu 5.454 total
python adam.py 4.82s user 0.66s system 99% cpu 5.492 total
python adam.py 4.92s user 0.70s system 99% cpu 5.637 total

kmaehashi · 2018-06-06T09:22:33Z

Could you check test failures?
Note that use of C++11 may break build on CentOS 6. Actually we once reverted to avoid this (Use std::map instead of std::unordered_map to avoid c++11 #732). It may be accepted as we now have wheels, though.

niboshi · 2018-06-07T01:18:03Z

cupy/core/elementwise.pxi

@@ -106,9 +111,14 @@ cdef _int_type = _int_iinfo.dtype.type

 cpdef _python_scalar_to_numpy_scalar(x):
    # Note that isinstance(x, six_integer_types) matches with bool.
-    if isinstance(x, bool):
+    typ = type(x)


Is it safe? x could be of some type derived from Python built-in scalars.

Previous code uses _python_type_to_numpy_type.

Do you mean we are not going to support such types?

OK, I will add fallback code.

(By the way, I'm not insisting we should support them. But if we are not going to support them, it's better to make consensus and write in the documentation.)

I found this condition.
Already, the input data is filtered by it.

I can't follow the link you pasted, but if there's assumption on this function, it's better to write in a comment.

Sorry, Please check this link.
https://github.com/okuta/cupy/blob/0a87a602e4b3a7276fd92bf753e94fcaa86288f2/cupy/core/elementwise.pxi#L152

okuta · 2018-06-13T11:04:18Z

I rebased.

chainer-ci · 2018-06-13T11:26:07Z

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

kmaehashi · 2018-06-13T11:30:55Z

Jenkins, test this please.

chainer-ci · 2018-06-13T15:15:05Z

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

chainer-ci · 2018-06-16T10:58:26Z

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

chainer-ci · 2018-06-17T05:25:23Z

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

okuta · 2018-06-17T05:56:57Z

Note that use of C++11 may break build on CentOS 6. Actually we once reverted to avoid this (#732). It may be accepted as we now have wheels, though.

@kmaehashi I change to use c++0x instead of c++11.

chainer-ci · 2018-06-17T07:29:03Z

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

niboshi · 2018-06-18T08:20:26Z

cupy/core/_scalar.pyx

+from cupy.core cimport internal
+
+
+cdef dict _typenames_base = {


Are these mapping safe?
e.g. can we assume long long == int64?

OK. I do not change this code.

niboshi · 2018-06-18T08:23:14Z

cupy/core/_scalar.pyx

+cpdef CScalar _python_scalar_to_c_scalar(x):
+    cdef CScalar ret = CScalar()
+    typ = type(x)
+    if x is bool:


x -> typ?

okuta · 2018-06-19T12:21:35Z

I fixed.

chainer-ci · 2018-06-19T13:01:24Z

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

chainer-ci · 2018-06-19T14:03:19Z

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

okuta · 2018-07-17T01:14:54Z

jenkins, test this please.

chainer-ci · 2018-07-17T02:17:45Z

Jenkins CI test (for commit 6920986) succeeded without errors!

toslunar

LGTM except for the line I commented.

toslunar · 2018-07-26T04:09:23Z

tests/cupy_tests/core_tests/test_internal.py

@@ -206,8 +208,30 @@ def test_invalid_stop_type(self):
    {'x': 2 ** 40,     'expect': 2 ** 40},
    {'x': 2 ** 40 - 1, 'expect': 2 ** 40},
    {'x': 2 ** 40 + 1, 'expect': 2 ** 41},
+    {'x': 2 ** 40 + 1, 'expect': 2 ** 41},


This set of parameters is already in the test.

toslunar · 2018-07-26T07:48:50Z

Jenkins, test this please.

chainer-ci · 2018-07-26T08:59:49Z

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

toslunar · 2018-07-26T09:20:29Z

The Jenkins failure ( tests.chainer_tests.functions_tests.normalization_tests.test_l2_normalization.TestL2Normalization_param_38.test_backward_cpu) is irrelevant to the PR.

Speed up ElementwiseKernel launch

chainer-ci · 2018-07-26T10:28:55Z

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

okuta · 2018-07-27T01:42:49Z

Thanks!

okuta added the cat:enhancement Improvements to existing features label Jun 3, 2018

okuta added this to the v5.0.0b2 milestone Jun 3, 2018

okuta changed the title ~~Introduce _CScalar type to up speed of ElementwiseKernel launching~~ Speed up ElementwiseKernel startup Jun 3, 2018

okuta changed the title ~~Speed up ElementwiseKernel startup~~ Speed up ElementwiseKernel launch Jun 3, 2018

okuta force-pushed the introduce-cscalar branch from d1e08c6 to 41dc8ca Compare June 4, 2018 04:07

niboshi reviewed Jun 7, 2018

View reviewed changes

okuta force-pushed the introduce-cscalar branch 3 times, most recently from 336b0e7 to e912c6e Compare June 13, 2018 10:52

okuta force-pushed the introduce-cscalar branch from e912c6e to 56cf755 Compare June 16, 2018 10:58

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

okuta force-pushed the introduce-cscalar branch 2 times, most recently from 39f0dd4 to 8367883 Compare June 17, 2018 05:55

niboshi requested changes Jun 18, 2018

View reviewed changes

okuta force-pushed the introduce-cscalar branch from 6c81df7 to d4e0210 Compare June 19, 2018 12:17

beam2d modified the milestones: v5.0.0b2, v5.0.0b3 Jun 21, 2018

okuta added 8 commits July 17, 2018 09:55

Fix bugs

a10a0b9

Fix assert

d07c278

Use ord

7366ecd

Avoid overflow error

f736383

Add test

611f0af

Fix

fc7931f

Fix test and modify function name

f9935ca

Set size and type

93d28b8

okuta force-pushed the introduce-cscalar branch from 033c464 to 93d28b8 Compare July 17, 2018 00:56

Fix

6920986

kmaehashi modified the milestones: v5.0.0b3, v5.0.0b4 Jul 18, 2018

toslunar reviewed Jul 26, 2018

View reviewed changes

Fix duplicated test

f51d899

toslunar approved these changes Jul 26, 2018

View reviewed changes

toslunar self-assigned this Jul 26, 2018

toslunar merged commit 2b809fa into cupy:master Jul 26, 2018

toslunar changed the title ~~Speed up ElementwiseKernel launch~~ Speed up ElementwiseKernel launch Jul 26, 2018

toslunar added a commit that referenced this pull request Jul 26, 2018

Merge pull request #1318 from okuta/introduce-cscalar

76ecd83

Speed up ElementwiseKernel launch

toslunar added a commit to toslunar/cupy that referenced this pull request Jul 26, 2018

Merge pull request cupy#1318 from okuta/introduce-cscalar

00169b8

Speed up ElementwiseKernel launch

toslunar mentioned this pull request Jul 26, 2018

[backport] Speed up ElementwiseKernel launch #1488

Merged

okuta deleted the introduce-cscalar branch July 27, 2018 01:42

kmaehashi mentioned this pull request Jul 31, 2018

Avoid compile error in old GCC (CentOS6) #1506

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Speed up ElementwiseKernel launch #1318

Speed up ElementwiseKernel launch #1318

okuta commented Jun 3, 2018

kmaehashi commented Jun 6, 2018

niboshi Jun 7, 2018

okuta Jun 7, 2018

niboshi Jun 7, 2018

okuta Jun 8, 2018

niboshi Jun 8, 2018

okuta Jun 9, 2018

niboshi Jun 11, 2018

okuta Jun 11, 2018

okuta commented Jun 13, 2018

chainer-ci commented Jun 13, 2018

kmaehashi commented Jun 13, 2018

chainer-ci commented Jun 13, 2018

chainer-ci commented Jun 16, 2018

chainer-ci commented Jun 17, 2018

okuta commented Jun 17, 2018

chainer-ci commented Jun 17, 2018

niboshi Jun 18, 2018

okuta Jun 18, 2018

niboshi Jun 18, 2018

okuta Jun 18, 2018

okuta commented Jun 19, 2018

chainer-ci commented Jun 19, 2018

chainer-ci commented Jun 19, 2018

okuta commented Jul 17, 2018

chainer-ci commented Jul 17, 2018

toslunar left a comment

toslunar Jul 26, 2018

okuta Jul 26, 2018

toslunar commented Jul 26, 2018

chainer-ci commented Jul 26, 2018

toslunar commented Jul 26, 2018

chainer-ci commented Jul 26, 2018

okuta commented Jul 27, 2018

		from cupy.core cimport internal


		cdef dict _typenames_base = {

Speed up ElementwiseKernel launch #1318

Speed up ElementwiseKernel launch #1318

Conversation

okuta commented Jun 3, 2018

kmaehashi commented Jun 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

okuta commented Jun 13, 2018

chainer-ci commented Jun 13, 2018

kmaehashi commented Jun 13, 2018

chainer-ci commented Jun 13, 2018

chainer-ci commented Jun 16, 2018

chainer-ci commented Jun 17, 2018

okuta commented Jun 17, 2018

chainer-ci commented Jun 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

okuta commented Jun 19, 2018

chainer-ci commented Jun 19, 2018

chainer-ci commented Jun 19, 2018

okuta commented Jul 17, 2018

chainer-ci commented Jul 17, 2018

toslunar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toslunar commented Jul 26, 2018

chainer-ci commented Jul 26, 2018

toslunar commented Jul 26, 2018

chainer-ci commented Jul 26, 2018

okuta commented Jul 27, 2018