Massive performance decrease in ListView when upgrading to LTS 1.532
Sandell, Robert
We recently dared and upgraded all our production masters from 1.480 to 1.532 after quite a long time of testing, so I realize that we might be a bit late to the party.
We started two weeks ago with one of our smaller masters (about 700 jobs) and let that run for two weeks to test the waters so to say, some “interesting” things happened during that time, but those are for another thread ;)
It should have been an alarm bell when we noticed a performance hit on a ListView we measure in graphite that basically includes all jobs.
A curl operation on that view went from taking 0.5 seconds to 5 seconds after the upgrade. In retrospect I shouldn’t have ignored that.
Now on our other servers that were upgraded yesterday we could see for the same curl on the other servers
From average 2 sec to 50 sec for a master with 2.5k jobs
And from average 2.5 sec to never going below 200 sec for the master with 6k jobs
It does not seem to be lazy loading related as most thread dumps that I do reveal the following stack trace:
Handling GET /view/All/ : http-8080-85
"Handling GET /view/All/ : http-8080-85" Id=4552 Group=main RUNNABLE
at java.lang.String$CaseInsensitiveComparator.compare(String.java:1170)
at java.lang.String.compareToIgnoreCase(String.java:1220)
at hudson.util.CaseInsensitiveComparator.compare(CaseInsensitiveComparator.java:40)
at hudson.util.CaseInsensitiveComparator.compare(CaseInsensitiveComparator.java:34)
at java.util.TreeMap.put(TreeMap.java:545)
at java.util.TreeSet.add(TreeSet.java:255)
at hudson.model.ListView.includeItems(ListView.java:214)
at hudson.model.ListView.getItems(ListView.java:164)
at hudson.model.ListView.getItems(ListView.java:60)
at hudson.Functions.getRelativeLinkTo(Functions.java:1014)
at sun.reflect.GeneratedMethodAccessor425.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at org.apache.commons.jexl.util.introspection.UberspectImpl$VelMethodImpl.invoke(UberspectImpl.java:258)
at org.apache.commons.jexl.parser.ASTMethod.execute(ASTMethod.java:104)
at org.apache.commons.jexl.parser.ASTReference.execute(ASTReference.java:83)
at org.apache.commons.jexl.parser.ASTReference.value(ASTReference.java:57)
at org.apache.commons.jexl.parser.ASTReferenceExpression.value(ASTReferenceExpression.java:51)
at org.apache.commons.jexl.ExpressionImpl.evaluate(ExpressionImpl.java:80)
at hudson.ExpressionFactory2$JexlExpression.evaluate(ExpressionFactory2.java:74)
As you can see no loading of builds, we disabled the weather column to relieve some of those problems.
I’ve been digging through the code, and not only is ListView.getItems quite complex (I lost count after 3n and 2 n log n) but when the index view has gotten all the items and renders the table it calls Functions.getRelativeLinkTo for every item in the list which in turn again calls ListView.getItems and viewItems.contains, so no wonder the view is rendering slow.
Now, one quick fix I could do would be to pass along the item list as a parameter to Functions.getRelativeLinkTo since the view already has the list and is iterating through it, but does anyone see anything else I could do as well, or any problems with that approach or my analysis?
Robert Sandell
Staff Engineer
Development Environment
Software Environment and Product Configuration
Sony Mobile Communications
Tel: +46 10 80 12721
Jesse Glick
https://issues.jenkins-ci.org/browse/JENKINS-18364
Also:
https://issues.jenkins-ci.org/browse/JENKINS-20052
Alas the current behavior of ListView.getAllItems is not at all
adequately tested, given the interacting possibilities of
· explicitly listed items
· recursive views
· item regexps
· status filtering
· ViewJobFilter’s, which can
· remove otherwise visible items
· add otherwise unmentioned or excluded items
· depend on, or even countermand, other filters
so making casual changes before a full test suite has been written
would be unwise.
Regarding the immediate problem, which is the excessive calls to
getRelativeLinkTo, I am wondering whether as I suggested in
https://issues.jenkins-ci.org/browse/JENKINS-19310
it would not be better to abandon ${jobBaseURL} and the complex code
which generates relative links, always emitting absolute links (that
is, absolute within the server: ${rootURL}/${item.url}), using
something similar to AbstractItem.getUrl to automatically include the
current (Ancestor) view in the link where applicable (so
/jenkins/job/someFolder/view/currentView/job/includedJob/).
Alternately, just hack getRelativeLinkTo to not call View.getItems at
all, and produce a URL under the assumption that the item is in the
ancestor view. Even if it were not, it would not actually matter
unless a View subclass overrode getItem(String) to verify that the
named item was a member—the default implementation does not do so, and
ListView does not override it. In other words, see if the following
helps and introduces no obvious problem:
diff --git a/core/src/main/java/hudson/Functions.java
b/core/src/main/java/hudson/Functions.java
index 4899606..3326e66 100644
--- a/core/src/main/java/hudson/Functions.java
+++ b/core/src/main/java/hudson/Functions.java
@@ -1014,25 +1014,14 @@ public class Functions {
Item i=p;
String url = "";
- Collection<TopLevelItem> viewItems;
- if (view != null) {
- viewItems = view.getItems();
- } else {
- viewItems = Collections.emptyList();
- }
while(true) {
ItemGroup ig = i.getParent();
url = i.getShortUrl()+url;
if(ig== Jenkins.getInstance() || (view != null && ig ==
view.getOwnerItemGroup())) {
assert i instanceof TopLevelItem;
- if(viewItems.contains((TopLevelItem)i)) {
- // if p and the current page belongs to the same
view, then return a relative path
- return normalizeURI(ancestors.get(view)+'/'+url);
- } else {
- // otherwise return a path from the root Hudson
- return
normalizeURI(request.getContextPath()+'/'+p.getUrl());
- }
+ // assume p and the current page belong to the same
view, so return a relative path
+ return normalizeURI(ancestors.get(view)+'/'+url);
}
path = ancestors.get(ig);
diff --git a/core/src/test/java/hudson/FunctionsTest.java
b/core/src/test/java/hudson/FunctionsTest.java
index a35cce9..c6ee303 100644
--- a/core/src/test/java/hudson/FunctionsTest.java
+++ b/core/src/test/java/hudson/FunctionsTest.java
@@ -42,6 +42,7 @@ import java.util.logging.LogRecord;
import jenkins.model.Jenkins;
+import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.jvnet.hudson.test.Bug;
@@ -135,6 +136,7 @@ public class FunctionsTest {
assertEquals("job/i/", result);
}
+ @Ignore("too expensive to make it correct")
@Test
@PrepareForTest({Stapler.class, Jenkins.class})
public void testGetRelativeLinkTo_JobNotContainedInView() throws Exception{
Sandell, Robert
diff --git a/core/src/main/java/hudson/Functions.java b/core/src/main/java/hudson/Functions.java
index 12a2c9b..fc38a02 100644
--- a/core/src/main/java/hudson/Functions.java
+++ b/core/src/main/java/hudson/Functions.java
@@ -1009,25 +1009,16 @@ public class Functions {
Item i=p;
String url = "";
- Collection<TopLevelItem> viewItems;
- if (view != null) {
- viewItems = view.getItems();
- } else {
- viewItems = Collections.emptyList();
- }
while(true) {
ItemGroup ig = i.getParent();
url = i.getShortUrl()+url;
+ String viewBaseUrl = ancestors.get(view);
+ if (viewBaseUrl == null) {
+ viewBaseUrl = request.getContextPath();
+ }
assert i instanceof TopLevelItem;
- if(viewItems.contains((TopLevelItem)i)) {
- // if p and the current page belongs to the same view, then return a relative path
- return normalizeURI(ancestors.get(view)+'/'+url);
- } else {
- // otherwise return a path from the root Hudson
- return normalizeURI(request.getContextPath()+'/'+p.getUrl());
- }
}
path = ancestors.get(ig);
The problem with the original suggestion was that if the ancestors doesn't contain a view, like on slave and label pages the url would be skewed.
I had it manually tested with ListView, inside a Sectioned View and inside a Nested View. And also on Personal views and Team Views.
But we are not using the folders plugin so there could be issues with that.
Robert Sandell
Tel: +46 10 80 12721
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Jesse Glick
<Robert....@sonymobile.com> wrote:
> The problem with the original suggestion was that if the ancestors doesn't contain a view, like on slave and label pages the url would be skewed.
different (simpler) patch.