Increase iterator performance for C++ modules

[SleepProgger]

To implement iterators on Objects one can bind the _iter_init _iter_next and _iter_get functions in C++ (and gdScript) at the moment.
When it comes to C++ this is slower as it has to be. (about 7 times slower in my tests).

I suggest adding a _script_use_iter flag and add the iter* as virtual functions to Object.
variant_op.cpp can then check in its iter* functions if the flag is set for the specific object and use the old (obj.call(...)) way if it is and the virtual function if not.
The _script_use_iter flag should be set to true by default, so everything behaves exactly the same except for Classes extending Object which set the flag to false and thus indicate that they want to handle iterators via the virtual functions. (The packed_data_container.cpp could benefit from it f.e.)

My proposal would look like this:
SleepProgger@c918699

I didn't sent a PR yet because i am not sure if i may have messed something up ;), and also if it might be a good idea to use a bitset instead of a simple boolean.
This way some _iter functions could be implemented/overwritten via script but not all (depending on the use case). This wouldn't bring any performance penalties compared to a boolean.
Anyone thoughts on that ?

I also did some benchmarks:
Test module and gd script: https://gist.github.com/SleepProgger/6ca1483dadc146f3b70ef8964aa2e581
Editor mode:

C++ impl iterator: Needed 325 ms for 1000000. 0.000325 ms per item.
C++ impl bound functions: Needed 2178 ms for 1000000. 0.002178 ms per item.
GDscript iterator: Needed 3579 ms for 1000000. 0.003579 ms per item.
Array: Needed 180 ms for 1000000. 0.000180 ms per item.

Release mode:

C++ impl iterator: Needed 87 ms for 1000000. 0.000087 ms per item.
C++ impl bound functions: Needed 888 ms for 1000000. 0.000888 ms per item.
GDscript iterator: Needed 1180 ms for 1000000. 0.001180 ms per item.
Array: Needed 60 ms for 1000000. 0.000060 ms per item.

[bojidar-bg]

Sounds good, though the _set_script_iter function would be a hassle..
What about the following:

1. Make Object::_init_iter init the scriptless iterator.
2. Make variant_op check if there is a script and if the script has the _init_iter method, use the script iterator for that object, otherwise use the cpp iterator.

That way we would handle both cases, abeit at a small runtime cost when starting the iteration (I guess).

Another option would be to set the _script_use_iter flag automatically (automagically) to true when the script has the iterator methods.

[SleepProgger]

Checking in _iter_init if the sript has iter* functions sound like a nice idea.

In fact before i changed it to the _set_script_iter function i did this in Object._post_initialize.
But that meant we'd have to check at every object instantiation even if it doesn't use iters at all.

The only remaining question for me is now: should we set a single boolean flag for all iter methods in variant_ops.iter_init or use a bitset (uint8_t should be enough) to save if the script has _iter_get and _iter_next ?
This would allow scripts to f.e. just implement the _iter_get function to format the results, or just implement iter_init and iter_next to implement filter. All in all i am not sure its worth the hassle and would go with the all or nothing variant.

Will send PR with boolean flag and checking in iter_init in some hours.

[bojidar-bg]

I think that the all-or-nothing variant is good enough -- though we would need to document it well 😃

[ghost]

First of all thank you for your report and sorry for the delay.

We released Godot 3.0 in January 2018 after 18 months of work, fixing many old issues either directly, or by obsoleting/replacing the features they were referring to.

We still have hundreds of issues whose relevance/reproducibility needs to be checked against the current stable version, and that's where you can help us.
Could you check if the issue that you described initially is still relevant/reproducible in Godot 3.0 or any newer version, and comment about it here?

For bug reports, please also make sure that the issue contains detailed steps to reproduce the bug and, if possible, a zipped project that can be used to reproduce it right away. This greatly speeds up debugging and bugfixing tasks for our contributors.

Our Bugsquad will review this issue more in-depth in 15 days, and potentially close it if its relevance could not be confirmed.

Thanks in advance.

Note: This message is being copy-pasted to many "stale" issues (90+ days without activity). It might happen that it is not meaningful for this specific issue or appears oblivious of the issue's context, if so please comment to notify the Bugsquad about it.

[akien-mga]

Our Bugsquad will review this issue more in-depth in 15 days, and potentially close it if its relevance could not be confirmed.

As there was no update since the previous post, we close this issue.

If it is still relevant in the way it was described and discussed above, please comment to ask for it to be reevaluated. If it is only partly relevant, or if the original issue has changed, it would be better to open a new issue (potentially referring to this one) focused on the current state.

[Xrayez]

If it is still relevant in the way it was described and discussed above, please comment to ask for it to be reevaluated. If it is only partly relevant, or if the original issue has changed, it would be better to open a new issue (potentially referring to this one) focused on the current state.

There's still a use case, see #42053 (the thread is quite old so decided to create a new issue for this).