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

Temporary solution for the 'Default' parameter value related performance issue. #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

valdemon
Copy link

Current 'default' parameter value approach leads into huge performance loss in the Jenkins UI,
when the default parameter values are calculated multiple times, eg. by other plugins
(like the inheritance plugin).
Anyway, one should consider if this approach is right, as the semantic of a 'default' value
is rather static, while the current approach assumes the default is the first element in the list, which is rather volatile value, by the nature and dynamic of the script logic behind it.
Maybe the 'default' value should be provided as an UI option, used by Jenkins when the script returns the 'null' value, what d'you think?
Proposed temporary solution is to return an empty string as a default, at least to prevent the performance loss.
The default values have no impact on my use cases, don't know if this would affect any others though.

@jenkinsadmin
Copy link
Member

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

@kinow
Copy link
Member

kinow commented Aug 19, 2015

Hi @valdemon! Thanks for the pull request. This week I'm in a different city and with limited Internet bandwidth, but next week I'll try to take a look at pull requests for the plug-in.

I had a quick look at this one now, and if this alleviates the performance problem, I'm +1 for it. However, keep in mind that we may have to think about maybe adding an option for this option. This way users could disable/enable this feature, and we would keep the existing behaviour.

Or we could just release a major version with this change as well :-)

Thanks a lot!

@imoutsatsos
Copy link
Member

Thanks for mentioning the inheritance plugin and bringing it to my attention @valdemon ! I am interested in this functionality since many of my project use a common set of parameters that could be inherited. Do you know whether the plugin supports multiple inheritance. i.e. inheriting parameters from more than one parent projects?

@valdemon
Copy link
Author

Hi,

@kinow : yes, it actually solves a real performance issue in my use case (inheritance plugin together with the Active Choices Plugin with most of the variables defined in the 'super' project.
Without this change it takes ages for Jenkins UI to refresh eg. on the project list view, when the view contains the 'super' project with variable definitions.
+1 for the option in AC plugin UI as I don't actually know the impact on another use cases (although I've got the feeling no user is actually aware of current 'default' handling, as one can actually only figure it out from the plugin code).

@imoutsatsos : as mentioned above I use the approach with the Inheritance plugin and your AC plugin together. This set allows me to:

  1. Dramatically reduce the number of Jenkins jobs - thanks to your plugin I have a single, lifecycle management job per module/service, which exist in a decent number as we go for a decoupled, microservice architecture with lots of reusable building blocks (modules, services)
  2. Concentrate the lifecycle management logic in single template 'superprojects', while the actual module jobs are only a inheritance declaration jobs.

Answering your question: yes, you are able to inherit from multiple projects, either by declaring the list of projects the one is inheriting from, or even create a nested inheritance, where project A inherits from B, which inherits from C. In both cases the sub-project inherits all parameter set from multiple parents.
You have to be aware though, that Inheritance plugin sorts the parameters by name, which can have an impact on inter-parameter dependencies. A simple solution is to name them properly ;)

@imoutsatsos
Copy link
Member

@valdemon Thanks for the feedback. Much appreciated, got to give it a try!

@kinow
Copy link
Member

kinow commented Nov 28, 2015

Note to self: review logging in fine to check why/when the method is called, and try to reproduce with inheritance plugin 😁

@kinow
Copy link
Member

kinow commented Mar 12, 2017

Note to self: maybe related to https://issues.jenkins-ci.org/browse/JENKINS-39593 (Performance slow in 1.5 release). Include reviewing this PR when debugging the issue.

@kinow
Copy link
Member

kinow commented May 27, 2018

Attached another hpi file to https://issues.jenkins-ci.org/browse/JENKINS-39593, which could prevent this performance issue from happening through a simple LRU cache. Testing needed.

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

Successfully merging this pull request may close these issues.

4 participants