Developer forums (C::B DEVELOPMENT STRICTLY!) > Development

small addition to editormanager - for convenience only

<< < (2/3) > >>

oBFusCATed:
This patch looks unacceptable to me. I don't like that every time GetBuiltinEditors is called one new vector is allocated.
For me this is a wasteful api that might be hard to change in the future if it turns out that this allocation is hurting performance.
If you want to do it you need to use some kind of an accessor class that implements begin/end and probably some kind of iterators.

frithjofh:
i don’t think there is any big performance hit, if any. consider it being used in a for ( x : GetBuiltinEditors() ) loop. the vector of pointers will be just here as temporary moved out of the function on return.

it is not a function that gets called many times and its share in execution time is negligible compared to the things that happen later in the function actually working on the pointers to the editors.

in the end it is not a bottleneck function anywhere in the code.

if it does hurt performance, any programmer is free to not use it and stay with the simpler functions, as of now

in places where performance is not the issue, or if it does not actually imply worse performance it will definitely be a more readable and less error prone way to express what the programmer wants.

but be it as it may. your the devs, you decide...  :)

frithjofh:
is this question decided already?

oBFusCATed:
Have you provided a better patch that fixes my concerns?
At the moment the only acceptable change I see is the addition of a const to one of the methods and it is partly acceptable, because it breaks the ABI.

frithjofh:
well. honestly, i think that you are perhaps overrating your concerns, no offense meant.

in the first place, the new function i provide is only a convenience function. so it doesn't harm performance, where performance is needed. where performance is needed, any concerned programmer can and will choose the underlying, and claimed to be faster, functions already in the API.

in the second place, if one takes a look at the typical function using this API functions, it goes more or less like this:

1) first it sets up a normal loop, calling on every iteration the GetEditorCount() function
2) the index is converted into a pointer to a built in editor by calling GetBuildinEditor() or similar
3) the pointer is checked against nullptr
4) then something is asked of the pointer like for instance GetName()
5) then that name is most likely put into an array or added to some wxwidget

in my (humble) opinion, the amount of execution time spend in 4) and 5) normally totally out-ways what is done in the rest of the loop. so, using the convenience function in this case would hurt performance only in a almost unnoticeable amount. in return it would reduce significantly the lines of code and checks performed over and over again in so many places in the code, with so many possibilities to forget something, or introduce an error...

i am in no way an expert, but only the fact, that i have to read less lines to grasp what a piece of code wants to achieve seems like a huge benefit, and as i stated above, in many of those calls, the code is not so performance critical as it seems. and as to resources: how many pointer would that vector have to hold? 10, 42, 128? it doesn't seem so wastefull at all: the vector gets allocated on stack for how much time, before it goes away?

but maybe i don't get the point, if so, please just ignore my proposition

Navigation

[0] Message Index

[#] Next page

[*] Previous page