dmpots wrote:

> Did you consider having something like `GetEnabledInstances` that returns a 
> vector with just the enabled instances? It seems like that would be a lot 
> less intrusive and I'd be surprised if this code is hot enough that the 
> overhead of creating a new vector instead of returning a reference makes a 
> difference.

We did consider this approach. There are some drawbacks beyond the performance 
issue, but the tradeoff may be worth it.

If we define the method like: `std::vector<PluginInstance> 
GetEnabledInstances()` then we are returning copies of the PluginInstance. 
Anything that modifies the internal state of the `PluginInstance` will be lost 
since it is operating on a copy (e.g. 
`GetEnabledInstances().front().DoSomethingToModifyInternalState()`).

We could also return pointers to the plugin instances 
like`std::vector<PluginInstance*> GetEnabledInstances()`. But these would be 
pointers to the locations in the vector stored in the `PluginInstance` class. 
So if a new plugin is added to the PluginInstances list the pointers could be 
invalidated. Probably not an issue with the current code, but could be unsafe 
to hand out those references in general.

I suppose another way would be to keep a 
`std::vector<shared_ptr<PluginInstance>>` and then creating a copy would not be 
an issue since any updates would also modify the original object.

Let me know what you think.

https://github.com/llvm/llvm-project/pull/132884
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to