Skip to content

Moved OpenCL memory allocation to runtime. #1958

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

Merged
merged 1 commit into from
Nov 8, 2018

Conversation

gcatron
Copy link
Contributor

@gcatron gcatron commented Nov 6, 2018

Description: Move OpenCL memory allocation to runtime. This is in support of #1904.
Testing: Ran ninja test all pass. run.sh --iterations=10 -opencl -time. Accuracy is the same as master now. Time is almost identical for all except resnet 50.
Master:
Wall time per iteration (s): 0.1335
---User Time--- --System Time-- --User+System-- ---Wall Time--- --- Name ---
1.1925 (100.0%) 0.1420 (100.0%) 1.3345 (100.0%) 1.3350 (100.0%) Infer
1.1925 (100.0%) 0.1420 (100.0%) 1.3345 (100.0%) 1.3350 (100.0%) Total
Wall time per iteration (s): 0.4013
Current Branch:
Wall time per iteration (s): 0.1628
---User Time--- --System Time-- --User+System-- ---Wall Time--- --- Name ---
0.8187 (100.0%) 0.1483 (100.0%) 0.9670 (100.0%) 1.6282 (100.0%) Infer
0.8187 (100.0%) 0.1483 (100.0%) 0.9670 (100.0%) 1.6282 (100.0%) Total
Wall time per iteration (s): 0.7370

Documentation: N/A
Progress on #1904

@rdzhabarov
Copy link
Contributor

Time is almost identical for all except resnet 50.

I suspect it has something to do with copying weights per inference onto the device.
What about vgg19 inference time?

Do you track anywhere migration plan for changing this eventually to copying weights once per inference?

@gcatron
Copy link
Contributor Author

gcatron commented Nov 7, 2018

Time is almost identical for all except resnet 50.

I suspect it has something to do with copying weights per inference onto the device.
What about vgg19 inference time?

Do you track anywhere migration plan for changing this eventually to copying weights once per inference?

vgg19 inference times were very similar between master and current branch. The migration is called out as a TODO in execution engine which points back to #1904 .

@gcatron gcatron changed the title [WIP] Moved OpenCL memory allocation to runtime. Moved OpenCL memory allocation to runtime. Nov 7, 2018
Copy link
Contributor

@opti-mix opti-mix left a comment

Choose a reason for hiding this comment

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

Looking good now! Please fix the small nits from my last comments and you're good to go.

@opti-mix
Copy link
Contributor

opti-mix commented Nov 8, 2018

@gcatron And do not forget to squash the commits if needed.

Created new BackendUtils library and put collectConstants there..
Added helper function to retrieve symbol offset by value from symbolTable
@gcatron gcatron force-pushed the opencl_backend_allocation_move branch from 682a785 to 7b7faf9 Compare November 8, 2018 17:23
@gcatron gcatron merged commit e8d9562 into pytorch:master Nov 8, 2018
@gcatron gcatron deleted the opencl_backend_allocation_move branch November 8, 2018 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants