Env amends by Ero-Sennin9 · Pull Request #436 · triton-inference-server/python_backend

Ero-Sennin9 · 2026-05-08T11:56:42Z

No description provided.

aleksn7 · 2026-05-12T07:27:19Z

-std::string
-EnvironmentManager::ExtractIfNotExtracted(std::string env_path)
+std::shared_ptr<Environment> // TODO: write logic with shared and weak ptrs in this method
+EnvironmentManager::GetEnvironment(ModelState* model_state)


Let’s preserve the old name

I think EnvironmentManager doesn't need to know about model_state

aleksn7 · 2026-05-12T07:28:18Z

-  const auto env_itr = env_map_.find(canonical_env_path);
+
+  std::string model_id = model_state.GetModelId();
+  std::string env_key =  model_state->Name() + "-" + model_id;


Why do we need to add model name here? Multiple models could refer to the same environment

aleksn7 · 2026-05-12T07:31:50Z

-    // need to be extracted again.
-    if (env_itr->second.second == last_modified_time) {
+    // need to be extracted again (or the current environment has no owners anymore).
+    if (env_itr->second.first.expired()) {


This condition is executed only if env has already been destroyed and model has to recreate it

Also, when deleting the environment, we want to remove its key from the map

In this case, we can’t derive the environment directory name from the map size, because that could cause environments to overwrite each other.

aleksn7 · 2026-05-12T07:36:56Z

+    if (env_itr->second.first.expired()) {
+      env_map_.erase(env_itr);
+    } else if (env_itr->second.second == last_modified_time) {
      env_extracted = true;


We could store last_modified_time in Environment structure

aleksn7 · 2026-05-12T07:43:57Z

         canonical_env_path)
            .c_str());
-    return canonical_env_path;
+    return std::make_shared<Environment>(canonical_env_path, canonical_env_path);


We should return nullptr here instead

aleksn7 · 2026-05-12T07:47:48Z

    std::string dst_env_path;
    if (re_extraction) {
-      dst_env_path = env_map_[canonical_env_path].first;
+      dst_env_path = env_map_[env_key].first;


I don't think this will compile
It's better to call the ->Path() method here

aleksn7 · 2026-05-12T07:50:18Z

 }
+
+Environment::Environment(const std::string& source, const std::string& path) : source_(source), path_(path) {
+  if (source == path) {


Let's check this condition before creating the object.
Also, in this case, destructor will be called with an invalid Path value

aleksn7 · 2026-05-12T07:50:57Z

 #include <mutex>
 #include <string>

+#include "python_be.h"


We don't need this include if we remove model_state from the GetEnvironment signature

aleksn7 · 2026-05-12T07:54:09Z

-        model_state->StateForBackend()->env_manager->ExtractIfNotExtracted(
-            python_execution_env_);
+    python_execution_env_ =
+        std::move(model_state->StateForBackend()->env_manager->GetEnvironment(model_state));


We don't need this std::move here because copy elision will work here

aleksn7 · 2026-05-12T14:59:10Z

-    if (re_extraction) {
-      // Just update the last modified timestamp
-      env_map_[canonical_env_path].second = last_modified_time;
+    if (re_extraction ) {


Let's merge this condition with the condition on line 308

aleksn7 · 2026-05-12T15:01:20Z

+      // Add the environment to the list of environments
+      env = std::make_shared<Environment>(
+        canonical_env_path_str, dst_env_path, last_modified_time);
+      env->SetManager(this);


Why do we need a separate method for this? Let's pass it into the Environment constructor

aleksn7 · 2026-05-12T15:08:26Z

+Environment::~Environment()
+{
+  if (manager_ != nullptr) {
+    manager_->env_map_.erase(source_);


This map is protected by a mutex inside manager_
Let's add a method inside EnvironmentManager struct that locks the mutex and erases environment from the map.

aleksn7 · 2026-05-12T15:08:54Z

+    const std::string& Source() const { return source_; }
+    const std::string& Path() const { return path_; }
+    const time_t& LastModifiedTime() const { return last_modified_time_; }
+    explicit operator std::string() const { return Path(); }


Useless operator

aleksn7 · 2026-05-13T07:38:47Z

-  const auto env_itr = env_map_.find(canonical_env_path);
+
+  std::string canonical_env_path_str(canonical_env_path);
+  std::string env_key = canonical_env_path_str;


Why do we need three identical variables canonical_env_path_str, env_key and canonical_env_path?

I would write something like this:

std::string canonical_env_path = [&]{ char canonical_env_path[PATH_MAX + 1]; char* err = realpath(env_path.c_str(), canonical_env_path); if (err == nullptr) { throw PythonBackendException( std::string("Failed to get the canonical path for ") + env_path + "."); } return std::string(canonical_env_path); }();

And drop the other variables: canonical_env_path_str, env_key

aleksn7 · 2026-05-13T08:24:35Z

+    // anymore).
+
+    if (env == nullptr) {
+      // refer to case when env was not loaded


We will enter this condition if an environment with such a key existed and was dropped earlier
Not because the environment hasn't been loaded yet

it simulates when env was not loaded, because we are deleting notice about it in map

I said: "refers to case", but not "this is case when"

aleksn7 · 2026-05-13T08:29:18Z

-EnvironmentManager::ExtractIfNotExtracted(std::string env_path)
+
+std::shared_ptr<
+    Environment>  // TODO: write logic with shared and weak ptrs in this method


Let's drop the useless comment here

aleksn7 · 2026-05-14T08:14:58Z

  }
-  const auto env_itr = env_map_.find(canonical_env_path);
+
+  std::string& env_key = canonical_env_path;


Do we really need this variable?

aleksn7 · 2026-05-14T12:52:09Z


-    std::string canonical_env_path_str(canonical_env_path);
+  if (!re_extraction) {
+    env->AddOwner();


We should increment reference count on each ExtractIfNotExtracted call

aleksn7 · 2026-05-14T12:54:07Z

  }
-  const auto env_itr = env_map_.find(canonical_env_path);
+
+  return EnvironmentGuard(*this, canonical_env_path);


Let's create environment here and pass it explicitly to the constructor

Denis Kolesnikov added 3 commits May 7, 2026 20:21

almostly finished fixing envs overfitting

e5de787

finished amends

25b2e25

added improvements

689333c

aleksn7 reviewed May 12, 2026

View reviewed changes

Denis Kolesnikov added 2 commits May 12, 2026 15:59

added little improves and also fixed ExtractIfNotExtracted

c0db109

added final amends to env logic

02e9357

aleksn7 reviewed May 12, 2026

View reviewed changes

Denis Kolesnikov added 2 commits May 12, 2026 18:38

added another fixes

5c4a52c

added final amends

d4a0c77

aleksn7 reviewed May 13, 2026

View reviewed changes

Denis Kolesnikov added 5 commits May 13, 2026 16:36

added last improvements to env code

f4c2fcd

fixed some compilation erros in env code

fcff38c

added another fixes to env code

d7b10fb

finished all compilation errors in env code

013684e

added numerous fixes to env code

913da36

aleksn7 reviewed May 14, 2026

View reviewed changes

Denis Kolesnikov added 4 commits May 14, 2026 14:06

deleted unuseful friend in env code

ce296e6

added guard principle to the env code

cb627f5

fixed stub launcher after previous remarks

9527d1c

added environment proxy to isolate Environment in EnvironmentManager

597c92d

aleksn7 reviewed May 14, 2026

View reviewed changes

Denis Kolesnikov added 3 commits May 14, 2026 16:40

added numerous compile time fixes to env code

20f86eb

added fixes to stub launcher code

be0d50f

added another fixes to stub launcher code

9eb0112

Denis Kolesnikov added 16 commits May 14, 2026 17:38

added another fixes to env code

70fb469

added another fixes

15a5cd5

added last fixes to the env code

9745dfd

added last fixes to the env code (i hope)

534f20c

add last fixes to the env code

d7658c5

added i hope last fixes to env code

9104caf

fixed signatures in env code

a72f975

added types fixes in the env code

3bf8ae1

fixed types in the env code

b58cee8

fixed EnvironmentProxy

aa90bd5

added fixes to EnvironmentProxy

a1c6a02

added fixes to EnvironmentProxy

b6d124c

fixed logs in the env code

0fb628b

added fixes to lgs in the env code

e270a41

returned logs code in the nev code

d9fbe30

fixed ref counting in the env code

91697dd

Conversation

Ero-Sennin9 commented May 8, 2026

Uh oh!

aleksn7 May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aleksn7 May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aleksn7 May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aleksn7 May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aleksn7 May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aleksn7 May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aleksn7 May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aleksn7 May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aleksn7 May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aleksn7 May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aleksn7 May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aleksn7 May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

aleksn7 May 12, 2026 •

edited

Loading

aleksn7 May 12, 2026 •

edited

Loading

aleksn7 May 12, 2026 •

edited

Loading

aleksn7 May 12, 2026 •

edited

Loading

aleksn7 May 12, 2026 •

edited

Loading

aleksn7 May 12, 2026 •

edited

Loading

aleksn7 May 12, 2026 •

edited

Loading

aleksn7 May 12, 2026 •

edited

Loading

aleksn7 May 12, 2026 •

edited

Loading

aleksn7 May 12, 2026 •

edited

Loading

aleksn7 May 13, 2026 •

edited

Loading

aleksn7 May 13, 2026 •

edited

Loading