Skip to content

refactor! : Update default workspace size based on platforms. #668

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
Oct 18, 2021

Conversation

peri044
Copy link
Collaborator

@peri044 peri044 commented Oct 18, 2021

Description

BREAKING CHANGE: This commit sets the default workspace size to 1GB for GPU platforms and 256MB for Jetson Nano/TX1 platforms whose compute capability is < 6.

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the style guidelines of this project (You can use the linters)
  • [z] I have performed a self-review of my own code
  • [z] I have commented my code, particularly in hard-to-understand areas and hacks
  • I have made corresponding changes to the documentation
  • I have added tests to verify my fix or my feature
  • [z] New and existing unit tests pass locally with my changes

@peri044 peri044 requested a review from narendasan October 18, 2021 16:54
@github-actions github-actions bot added component: conversion Issues re: Conversion stage component: core Issues re: The core compiler labels Oct 18, 2021
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

There are some changes that do not conform to C++ style guidelines:

diff --git a/workspace/core/conversion/conversionctx/ConversionCtx.cpp b/tmp/changes.txt
index 3c98207..62a2cfa 100644
--- a/workspace/core/conversion/conversionctx/ConversionCtx.cpp
+++ b/tmp/changes.txt
@@ -1,8 +1,8 @@
#include "core/conversion/conversionctx/ConversionCtx.h"
#include <cuda_runtime.h>
-#include <typeinfo>
#include <iostream>
#include <sstream>
+#include <typeinfo>
#include <utility>

namespace trtorch {
@@ -65,8 +65,8 @@ ConversionCtx::ConversionCtx(BuilderSettings build_settings)
  cudaGetDeviceProperties(&device_prop, settings.device.gpu_id);
  // GPU default WS size : 1 GB
  // Jetson nano compute capability is 5.X. WS = 16 MB
-  if (build_settings.workspace_size == 0){
-    if (device_prop.major < 6){
+  if (build_settings.workspace_size == 0) {
+    if (device_prop.major < 6) {
      settings.workspace_size = 256 * (1 << 20);
    } else {
      settings.workspace_size = 1 << 30;
ERROR: Some files do not conform to style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

There are some changes that do not conform to C++ style guidelines:

diff --git a/workspace/core/conversion/conversionctx/ConversionCtx.cpp b/tmp/changes.txt
index 3c98207..62a2cfa 100644
--- a/workspace/core/conversion/conversionctx/ConversionCtx.cpp
+++ b/tmp/changes.txt
@@ -1,8 +1,8 @@
#include "core/conversion/conversionctx/ConversionCtx.h"
#include <cuda_runtime.h>
-#include <typeinfo>
#include <iostream>
#include <sstream>
+#include <typeinfo>
#include <utility>

namespace trtorch {
@@ -65,8 +65,8 @@ ConversionCtx::ConversionCtx(BuilderSettings build_settings)
  cudaGetDeviceProperties(&device_prop, settings.device.gpu_id);
  // GPU default WS size : 1 GB
  // Jetson nano compute capability is 5.X. WS = 16 MB
-  if (build_settings.workspace_size == 0){
-    if (device_prop.major < 6){
+  if (build_settings.workspace_size == 0) {
+    if (device_prop.major < 6) {
      settings.workspace_size = 256 * (1 << 20);
    } else {
      settings.workspace_size = 1 << 30;
ERROR: Some files do not conform to style guidelines

Copy link
Contributor

@andi4191 andi4191 left a comment

Choose a reason for hiding this comment

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

Minor comment

@@ -120,6 +135,7 @@ ConversionCtx::ConversionCtx(BuilderSettings build_settings)
cfg->setMinTimingIterations(settings.num_min_timing_iters);
cfg->setAvgTimingIterations(settings.num_avg_timing_iters);
cfg->setMaxWorkspaceSize(settings.workspace_size);

Copy link
Contributor

Choose a reason for hiding this comment

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

@peri044 : Can we remove this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done 👍

@@ -120,6 +135,7 @@ ConversionCtx::ConversionCtx(BuilderSettings build_settings)
cfg->setMinTimingIterations(settings.num_min_timing_iters);
cfg->setAvgTimingIterations(settings.num_avg_timing_iters);
cfg->setMaxWorkspaceSize(settings.workspace_size);

Copy link
Contributor

Choose a reason for hiding this comment

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

@peri044 : Can we remove this? Is this for readability?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

There are some changes that do not conform to C++ style guidelines:

diff --git a/workspace/core/compiler.cpp b/tmp/changes.txt
index 0069598..69159f7 100644
--- a/workspace/core/compiler.cpp
+++ b/tmp/changes.txt
@@ -355,7 +355,7 @@ torch::jit::script::Module CompileGraph(const torch::jit::script::Module& mod, C
  cudaDeviceProp device_prop;
  cudaGetDeviceProperties(&device_prop, device_spec.gpu_id);
  if (workspace_size == 0) {
-    if (device_prop.major < 6){
+    if (device_prop.major < 6) {
      cfg.convert_info.engine_settings.workspace_size = 256 * (1 << 20);
    } else {
      cfg.convert_info.engine_settings.workspace_size = 1 << 30;
diff --git a/workspace/core/conversion/conversionctx/ConversionCtx.cpp b/tmp/changes.txt
index 340724f..1dfd9fd 100644
--- a/workspace/core/conversion/conversionctx/ConversionCtx.cpp
+++ b/tmp/changes.txt
@@ -1,8 +1,8 @@
#include "core/conversion/conversionctx/ConversionCtx.h"
#include <cuda_runtime.h>
-#include <typeinfo>
#include <iostream>
#include <sstream>
+#include <typeinfo>
#include <utility>

namespace trtorch {
ERROR: Some files do not conform to style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

There are some changes that do not conform to C++ style guidelines:

diff --git a/workspace/core/compiler.cpp b/tmp/changes.txt
index dd4e716..f947e5c 100644
--- a/workspace/core/compiler.cpp
+++ b/tmp/changes.txt
@@ -355,7 +355,7 @@ torch::jit::script::Module CompileGraph(const torch::jit::script::Module& mod, C
  cudaDeviceProp device_prop;
  cudaGetDeviceProperties(&device_prop, device_spec.gpu_id);
  if (workspace_size == 0) {
-    if (device_prop.major < 6){
+    if (device_prop.major < 6) {
      cfg.convert_info.engine_settings.workspace_size = 256 * (1 << 20);
    } else {
      cfg.convert_info.engine_settings.workspace_size = 1 << 30;
ERROR: Some files do not conform to style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

@peri044 peri044 force-pushed the ws_size branch 2 times, most recently from 03c0536 to 391a4c0 Compare October 18, 2021 19:00
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

BREAKING CHANGE: This commit sets the default workspace size to 1GB for GPU platforms and 256MB for Jetson Nano/TX1 platforms whose compute capability is < 6.

Signed-off-by: Dheeraj Peri <[email protected]>

Signed-off-by: Dheeraj Peri <[email protected]>

Signed-off-by: Dheeraj Peri <[email protected]>

Signed-off-by: Dheeraj Peri <[email protected]>

Signed-off-by: Dheeraj Peri <[email protected]>
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

@andi4191 andi4191 merged commit 4d95b04 into master Oct 18, 2021
@peri044 peri044 deleted the ws_size branch November 16, 2021 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: conversion Issues re: Conversion stage component: core Issues re: The core compiler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants