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

[RPC] microtvm: fix RPC large transfer size issue #7838

Merged
merged 12 commits into from
Apr 22, 2021
Merged

Conversation

mehrdadh
Copy link
Member

@mehrdadh mehrdadh commented Apr 13, 2021

This PR:

  • Changes CopyToRemote and CopyFromRemote functions to handle smaller buffer size on hosts like microTVM devices.
  • Adds a global packed function to set the RPC max transfer size.
  • Increases memory size for CRT host main file to handle larger models.

@mehrdadh mehrdadh force-pushed the rpc_fix branch 4 times, most recently from ef8f193 to eca259a Compare April 14, 2021 17:47
@mehrdadh mehrdadh marked this pull request as ready for review April 14, 2021 18:10
@mehrdadh
Copy link
Member Author

cc @areusch

@mehrdadh
Copy link
Member Author

cc @tqchen

Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

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

thanks @mehrdadh! mostly minor things here, but one question on what exactly we should expose over RPC.

src/runtime/rpc/rpc_endpoint.cc Outdated Show resolved Hide resolved
src/runtime/rpc/rpc_endpoint.cc Outdated Show resolved Hide resolved
src/runtime/rpc/rpc_endpoint.cc Outdated Show resolved Hide resolved
src/runtime/crt/common/crt_runtime_api.c Outdated Show resolved Hide resolved
src/runtime/crt/host/crt_config.h Outdated Show resolved Hide resolved
src/runtime/crt/host/main.cc Outdated Show resolved Hide resolved
src/runtime/rpc/rpc_endpoint.cc Outdated Show resolved Hide resolved
src/runtime/rpc/rpc_endpoint.cc Outdated Show resolved Hide resolved
src/runtime/rpc/rpc_endpoint.cc Outdated Show resolved Hide resolved
src/runtime/rpc/rpc_endpoint.h Outdated Show resolved Hide resolved
@mehrdadh mehrdadh force-pushed the rpc_fix branch 3 times, most recently from 7428d0c to 447aa46 Compare April 15, 2021 00:49
@mehrdadh mehrdadh changed the title [RPC] microtvm: fix rpc large transfer size for microtvm targets [RPC] microtvm: fix RPC large transfer size issue Apr 16, 2021
Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

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

thanks @mehrdadh, this is looking good! just a few minor clarifications on comments, and please look at the unit test failure

src/runtime/crt/host/main.cc Outdated Show resolved Hide resolved
src/runtime/rpc/rpc_endpoint.cc Outdated Show resolved Hide resolved
src/runtime/rpc/rpc_endpoint.cc Outdated Show resolved Hide resolved
src/runtime/rpc/rpc_endpoint.h Show resolved Hide resolved
@mehrdadh
Copy link
Member Author

@areusch I think this is probably a CI bug?

@areusch
Copy link
Contributor

areusch commented Apr 21, 2021

@mehrdadh yeah it looks like a poorly-written test to me. looks like on the retrigger, it passed. see also #7870

@mehrdadh
Copy link
Member Author

@areusch PTAL.
Thanks!

Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

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

thanks @mehrdadh!

@areusch areusch merged commit 060e001 into apache:main Apr 22, 2021
mehrdadh added a commit to mehrdadh/tvm that referenced this pull request Apr 22, 2021
* fix rpc for microtvm

* apply feedbacks

* bundle deploy fix

* fix func registry size

* mv constant

* fix copyfromremote

* address comments and fix error

* change rpc default max size

* Trigger Build

* add checks

* Trigger Build

* fix ICHECK
echuraev pushed a commit to echuraev/tvm that referenced this pull request Apr 29, 2021
* fix rpc for microtvm

* apply feedbacks

* bundle deploy fix

* fix func registry size

* mv constant

* fix copyfromremote

* address comments and fix error

* change rpc default max size

* Trigger Build

* add checks

* Trigger Build

* fix ICHECK
umangyadav pushed a commit to umangyadav/tvm that referenced this pull request May 5, 2021
* fix rpc for microtvm

* apply feedbacks

* bundle deploy fix

* fix func registry size

* mv constant

* fix copyfromremote

* address comments and fix error

* change rpc default max size

* Trigger Build

* add checks

* Trigger Build

* fix ICHECK
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021
* fix rpc for microtvm

* apply feedbacks

* bundle deploy fix

* fix func registry size

* mv constant

* fix copyfromremote

* address comments and fix error

* change rpc default max size

* Trigger Build

* add checks

* Trigger Build

* fix ICHECK
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021
* fix rpc for microtvm

* apply feedbacks

* bundle deploy fix

* fix func registry size

* mv constant

* fix copyfromremote

* address comments and fix error

* change rpc default max size

* Trigger Build

* add checks

* Trigger Build

* fix ICHECK
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021
* fix rpc for microtvm

* apply feedbacks

* bundle deploy fix

* fix func registry size

* mv constant

* fix copyfromremote

* address comments and fix error

* change rpc default max size

* Trigger Build

* add checks

* Trigger Build

* fix ICHECK
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request May 11, 2021
* fix rpc for microtvm

* apply feedbacks

* bundle deploy fix

* fix func registry size

* mv constant

* fix copyfromremote

* address comments and fix error

* change rpc default max size

* Trigger Build

* add checks

* Trigger Build

* fix ICHECK
@mehrdadh mehrdadh deleted the rpc_fix branch April 11, 2022 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants