Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OC4 Stop reconstructing controllers on each call #8920

Closed
wants to merge 1 commit into from
Closed

OC4 Stop reconstructing controllers on each call #8920

wants to merge 1 commit into from

Conversation

stalker780
Copy link
Contributor

Fix controllers calls:

  1. Allows storing variables inside controller classes.
  2. Also most 3rd party extensions call lots of init functions on their controllers __construct(). This may cause significant performance loss which is hard to debug.
  3. Event system would make things only worse, calling more controller functions and reconstructing controller on each call.

Fix controllers calls:
1. Allows storing variables inside controller classes.
2. Also most 3rd party extensions call lots of init functions on their controllers __construct(). This may cause significant performance loss which is hard to debug.
3. Event system would make things only worse, calling more controller functions and reconstructing controller on each call.
@straightlight
Copy link
Contributor

So what do you suggest? To only stick to core controllers as an alternative to use extensions then?

@stalker780
Copy link
Contributor Author

stalker780 commented Nov 21, 2020

I have pulled my suggestion.
It checks if controller was constructed previously and takes it from registry with all it's variables.

As far as I remember we discussed this here with Daniel since OC 2.x versions. But this behavior is still there in core.

@straightlight
Copy link
Contributor

That also means that all codes at the same time, including 3rd party, would be loaded into the Engine.

As far as I remember we discussed this here with Daniel since OC 2.x versions. But this behavior is still there in core.

That might be because the suggestion wasn't considered at the time.

@stalker780
Copy link
Contributor Author

That also means that all codes at the same time, including 3rd party, would be loaded into the Engine.

There is no problem loading all models and libraries, so why loading controllers should be an issue?
Isn't their multiple reconstruction a bigger problem for CPU and disk?

@mhcwebdesign
Copy link
Collaborator

Different controller instances, even of the same controller class, may have different variables. It won't be a straight-forward shot with a simple core engine change.

@straightlight
Copy link
Contributor

Agreed.

@danielkerr
Copy link
Member

not required

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants