Skip to content

Commit 9bf418f

Browse files
committed
enh: The optimization eliminates the overhead of:
Forking a new Python process (~1.2ms per fork_exec) Initializing the Python interpreter Loading modules and dependencies Setting up the subprocess communication pipes Signed-off-by: habeck <habeck@us.ibm.com>
1 parent 3c4720b commit 9bf418f

6 files changed

Lines changed: 601 additions & 114 deletions

File tree

cpex/framework/isolated/client.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,13 @@ async def initialize(self) -> None:
223223
else:
224224
logger.info("Using cached venv, skipping requirements installation")
225225

226+
async def cleanup(self) -> None:
227+
"""Cleanup resources, including stopping the worker process."""
228+
if self.comm:
229+
logger.info("Stopping worker process for plugin '%s'", self.name)
230+
self.comm.stop_worker()
231+
self.comm = None
232+
226233
def _validate_hook_invocation(self, hook_type: str) -> type[PluginResult]:
227234
"""Validate hook type and communication channel.
228235

cpex/framework/isolated/venv_comm.py

Lines changed: 188 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,19 @@
1111
import os
1212
import subprocess
1313
import sys
14+
import threading
15+
import uuid
1416
from pathlib import Path
15-
from typing import Any
17+
from queue import Empty, Queue
18+
from typing import Any, Optional
1619

1720
import orjson
1821

1922
logger = logging.getLogger(__name__)
2023

2124

2225
class VenvProcessCommunicator:
23-
"""Handles communication with child processes in different virtual environments."""
26+
"""Handles communication with a long-running child process in a different virtual environment."""
2427

2528
def __init__(self, venv_path: str) -> None:
2629
"""
@@ -31,6 +34,12 @@ def __init__(self, venv_path: str) -> None:
3134
"""
3235
self.venv_path = Path(venv_path)
3336
self.python_executable = self._get_python_executable()
37+
self.process: Optional[subprocess.Popen] = None
38+
self.reader_thread: Optional[threading.Thread] = None
39+
self.stderr_thread: Optional[threading.Thread] = None
40+
self.response_queues: dict[str, Queue] = {}
41+
self.lock = threading.Lock()
42+
self.running = False
3443
logger.info("cwd: %s", os.getcwd())
3544

3645
def _get_python_executable(self):
@@ -57,47 +66,199 @@ def install_requirements(self, requirements_file: str) -> None:
5766
if rc != 0:
5867
raise Exception(f"Failed to install requirements from {requirements_file}")
5968

60-
def send_task(self, script_path: str, task_data: Any) -> Any:
69+
def start_worker(self, script_path: str) -> None:
6170
"""
62-
Send a task to child process and get response.
71+
Start the long-running worker process.
6372
6473
Args:
65-
script_path (str): Path to the child script
66-
task_data (dict): Data to send to child process
67-
68-
Returns:
69-
dict: Response from child process
74+
script_path (str): Path to the worker script
7075
"""
71-
process = None
76+
if self.running:
77+
logger.warning("Worker process already running")
78+
return
79+
7280
try:
73-
# Prepare input data as JSON
74-
input_json = orjson.dumps(task_data).decode()
7581
# Start child process
76-
process = subprocess.Popen(
82+
self.process = subprocess.Popen(
7783
[self.python_executable, script_path],
7884
stdin=subprocess.PIPE,
7985
stdout=subprocess.PIPE,
8086
stderr=subprocess.PIPE,
8187
text=True,
82-
cwd=os.getcwd(), # Maintain current working directory
88+
bufsize=1, # Line buffered
89+
cwd=os.getcwd(),
8390
)
8491

85-
# Send data and get response
86-
stdout, stderr = process.communicate(input=input_json, timeout=30)
92+
self.running = True
93+
94+
# Start reader thread to handle responses
95+
self.reader_thread = threading.Thread(target=self._read_responses, daemon=True)
96+
self.reader_thread.start()
97+
98+
# Start stderr reader thread to capture errors
99+
self.stderr_thread = threading.Thread(target=self._read_stderr, daemon=True)
100+
self.stderr_thread.start()
101+
102+
logger.info("Worker process started with PID: %s", self.process.pid)
103+
104+
except Exception as e:
105+
self.running = False
106+
raise RuntimeError(f"Failed to start worker process: {e}")
107+
108+
def _read_stderr(self) -> None:
109+
"""Background thread to read and log stderr from worker process."""
110+
if not self.process or not self.process.stderr:
111+
return
112+
113+
while self.running and self.process and self.process.stderr:
114+
try:
115+
line = self.process.stderr.readline()
116+
if not line:
117+
break
118+
# Log stderr output from worker
119+
logger.debug("Worker stderr: %s", line.strip())
120+
except Exception as e:
121+
logger.error("Error reading stderr: %s", e)
122+
break
123+
124+
def _read_responses(self) -> None:
125+
"""Background thread to read responses from worker process."""
126+
while self.running and self.process and self.process.stdout:
127+
try:
128+
line = self.process.stdout.readline()
129+
if not line:
130+
# Process has terminated
131+
logger.warning("Worker process stdout closed")
132+
break
133+
134+
line = line.strip()
135+
if not line:
136+
# Empty line, skip
137+
continue
138+
139+
try:
140+
response = json.loads(line)
141+
request_id = response.get("request_id")
142+
143+
if request_id:
144+
with self.lock:
145+
if request_id in self.response_queues:
146+
self.response_queues[request_id].put(response)
147+
logger.debug("Response queued for request_id: %s", request_id)
148+
else:
149+
logger.warning("Received response for unknown request_id: %s", request_id)
150+
else:
151+
logger.warning("Received response without request_id: %s", line[:100])
152+
153+
except json.JSONDecodeError as e:
154+
logger.error("Failed to decode response: %s, line: %s", e, line[:200])
155+
156+
except Exception as e:
157+
logger.exception("Error reading response: %s", e)
158+
break
159+
160+
self.running = False
161+
logger.info("Response reader thread terminated")
162+
163+
def send_task(self, script_path: str, task_data: Any, timeout: float = 30.0) -> Any:
164+
"""
165+
Send a task to the long-running worker process and get response.
166+
167+
Args:
168+
script_path (str): Path to the child script (used for worker initialization)
169+
task_data (dict): Data to send to child process
170+
timeout (float): Timeout in seconds for waiting for response
171+
172+
Returns:
173+
dict: Response from child process
174+
"""
175+
# Start worker if not running
176+
if not self.running:
177+
self.start_worker(script_path)
178+
179+
# Generate unique request ID
180+
request_id = str(uuid.uuid4())
181+
task_data["request_id"] = request_id
182+
183+
# Create response queue for this request
184+
response_queue: Queue = Queue()
185+
with self.lock:
186+
self.response_queues[request_id] = response_queue
87187

88-
if process.returncode != 0:
89-
raise RuntimeError(f"Child process failed: {stderr}")
188+
try:
189+
# Send task to worker
190+
input_json = orjson.dumps(task_data).decode()
191+
if self.process and self.process.stdin:
192+
self.process.stdin.write(input_json + "\n")
193+
self.process.stdin.flush()
194+
else:
195+
raise RuntimeError("Worker process stdin not available")
90196

91-
# Parse response
197+
# Wait for response
92198
try:
93-
response = json.loads(stdout.strip())
199+
response = response_queue.get(timeout=timeout)
200+
201+
# Check for errors in response
202+
if response.get("status") == "error":
203+
raise RuntimeError(f"Worker process error: {response.get('message')}")
204+
205+
# Remove request_id from response before returning
206+
response.pop("request_id", None)
94207
return response
95-
except json.JSONDecodeError:
96-
raise RuntimeError(f"Invalid JSON response from child: {stdout}")
97208

98-
except subprocess.TimeoutExpired:
99-
if process:
100-
process.kill()
101-
raise RuntimeError("Child process timed out")
209+
except Empty:
210+
raise RuntimeError(f"Worker process timed out after {timeout} seconds")
211+
212+
finally:
213+
# Clean up response queue
214+
with self.lock:
215+
self.response_queues.pop(request_id, None)
216+
217+
def stop_worker(self) -> None:
218+
"""Stop the long-running worker process."""
219+
if not self.running:
220+
return
221+
222+
self.running = False
223+
224+
try:
225+
if self.process:
226+
# Send shutdown signal
227+
if self.process.stdin:
228+
try:
229+
shutdown_task = {"task_type": "shutdown", "request_id": "shutdown"}
230+
self.process.stdin.write(json.dumps(shutdown_task) + "\n")
231+
self.process.stdin.flush()
232+
except Exception as e:
233+
logger.warning("Failed to send shutdown signal: %s", e)
234+
235+
# Wait for process to terminate gracefully
236+
try:
237+
self.process.wait(timeout=5.0)
238+
except subprocess.TimeoutExpired:
239+
logger.warning("Worker process did not terminate gracefully, killing it")
240+
self.process.kill()
241+
self.process.wait()
242+
243+
logger.info("Worker process stopped")
244+
102245
except Exception as e:
103-
raise RuntimeError(f"Communication error: {e}")
246+
logger.error("Error stopping worker process: %s", e)
247+
248+
finally:
249+
self.process = None
250+
if self.reader_thread and self.reader_thread.is_alive():
251+
self.reader_thread.join(timeout=2.0)
252+
self.reader_thread = None
253+
if self.stderr_thread and self.stderr_thread.is_alive():
254+
self.stderr_thread.join(timeout=2.0)
255+
self.stderr_thread = None
256+
257+
def is_alive(self) -> bool:
258+
"""Check if the worker process is alive and running."""
259+
return self.running and self.process is not None and self.process.poll() is None
260+
261+
def __del__(self):
262+
"""Cleanup when object is destroyed."""
263+
if hasattr(self, 'running'):
264+
self.stop_worker()

cpex/framework/isolated/worker.py

Lines changed: 62 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -100,24 +100,70 @@ async def process_task(task_data):
100100

101101

102102
async def main():
103-
"""Main function - read from stdin, process, write to stdout."""
103+
"""Main function - continuously read from stdin, process tasks, write to stdout."""
104+
logger.info("Worker process started, waiting for tasks...")
105+
104106
try:
105-
# Read input from parent process
106-
input_data = sys.stdin.read()
107-
task_data = json.loads(input_data)
108-
109-
# Process the task
110-
response = await process_task(task_data)
111-
serializable_response = response.model_dump(mode="json") if response else None
112-
# Send response back to parent
113-
print(json.dumps(serializable_response))
114-
115-
except json.JSONDecodeError:
116-
error_response = {"status": "error", "message": "Invalid JSON input"}
117-
print(json.dumps(error_response))
107+
# Continuously read and process tasks
108+
while True:
109+
try:
110+
# Read one line at a time
111+
line = sys.stdin.readline()
112+
113+
# Check for EOF
114+
if not line:
115+
logger.info("EOF received, shutting down worker")
116+
break
117+
118+
# Parse the task
119+
task_data = json.loads(line.strip())
120+
request_id = task_data.get("request_id", "unknown")
121+
122+
# Check for shutdown signal
123+
if task_data.get("task_type") == "shutdown":
124+
logger.info("Shutdown signal received")
125+
response = {"status": "success", "message": "Shutting down", "request_id": request_id}
126+
print(json.dumps(response), flush=True)
127+
break
128+
129+
# Process the task
130+
response = await process_task(task_data)
131+
132+
# Serialize response
133+
if response:
134+
serializable_response = response.model_dump(mode="json")
135+
else:
136+
serializable_response = {"status": "success"}
137+
138+
# Add request_id to response
139+
serializable_response["request_id"] = request_id
140+
141+
# Send response back to parent (one line per response)
142+
print(json.dumps(serializable_response), flush=True)
143+
144+
except json.JSONDecodeError as e:
145+
error_response = {
146+
"status": "error",
147+
"message": f"Invalid JSON input: {str(e)}",
148+
"request_id": task_data.get("request_id", "unknown") if 'task_data' in locals() else "unknown"
149+
}
150+
print(json.dumps(error_response), flush=True)
151+
152+
except Exception as e:
153+
logger.error("Error processing task: %s", str(e))
154+
error_response = {
155+
"status": "error",
156+
"message": f"Unexpected error: {str(e)}",
157+
"request_id": task_data.get("request_id", "unknown") if 'task_data' in locals() else "unknown"
158+
}
159+
print(json.dumps(error_response), flush=True)
160+
161+
except KeyboardInterrupt:
162+
logger.info("Worker interrupted")
118163
except Exception as e:
119-
error_response = {"status": "error", "message": f"Unexpected error: {str(e)}"}
120-
print(json.dumps(error_response))
164+
logger.exception("Fatal error in worker main loop")
165+
finally:
166+
logger.info("Worker process shutting down")
121167

122168

123169
if __name__ == "__main__":

tests/unit/cpex/framework/isolated/test_client.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -650,6 +650,25 @@ async def test_initialize_with_invalid_cache(self, mock_save_metadata, mock_cach
650650
# Should install requirements when cache is invalid
651651
mock_comm.install_requirements.assert_called_once()
652652
mock_save_metadata.assert_called_once()
653+
@pytest.mark.asyncio
654+
async def test_cleanup(self, plugin):
655+
"""Test cleanup method stops worker process."""
656+
mock_comm = MagicMock()
657+
plugin.comm = mock_comm
658+
659+
await plugin.cleanup()
660+
661+
mock_comm.stop_worker.assert_called_once()
662+
assert plugin.comm is None
663+
664+
@pytest.mark.asyncio
665+
async def test_cleanup_no_comm(self, plugin):
666+
"""Test cleanup when comm is None."""
667+
plugin.comm = None
668+
669+
# Should not raise error
670+
await plugin.cleanup()
671+
653672

654673

655674
# Made with Bob

0 commit comments

Comments
 (0)