containers / youki

A container runtime written in Rust
https://containers.github.io/youki/
Apache License 2.0
6.2k stars 337 forks source link

Use HashMap for envs in container_init_process #2817

Closed musaprg closed 3 months ago

musaprg commented 3 months ago

The environment variables envs for the container processes are obtained in Vec<String> and manipulated based on the various circumstances in container_init_process^manipulate-envs-1. They're eventually converted into the HashMap, so the logic to manipulate environment variables should be performed for the parsed HashMap instead of Vec<String>. It would reduce the conditional logic based on the string match^conditional-logic-code.

There might be a downside to applying HashMap for manipulation, for example, inserting a new value could be slow. AFAIU, there are a few insertions in the current container_process_init function, so I don't think it would put the additional latency for the initialization process.

yihuaf commented 3 months ago

There might be a downside to applying HashMap for manipulation, for example, inserting a new value could be slow. AFAIU, there are a few insertions in the current container_process_init function, so I don't think it would put the additional latency for the initialization process.

The improvements looks to be worth it. I would not be too concerned with the performance of insert with Hashmap vs. Array. For this specific usecase.