Optimization to reduce facet "index" search on every request #97
Conversation
|
PTAL @vivek |
| @@ -13,7 +13,7 @@ pipeline { | |||
| } | |||
| } | |||
|
|
|||
| postBuild { | |||
| post { | |||
There was a problem hiding this comment.
Unrelated change to make CI pass. Needs #93 to be merged.
|
From what I can tell the test that is failing on CI is intermittent |
|
@i386 right, it needs core upgrade. Maybe we can put the code related to PR 77 feature in separate plugin that require upgraded core - this way everything else would continue working, only this parallel routing implementation plugin will need later core version. Something to investigate though. |
|
If only what was done in #77 was a plugable thing :( |
|
I haven't look at this change yet, but we need to make sure this change doesn't have compatibility implications. |
|
@kohsuke what would be the best way to look at doing this? |
|
I looked at this change more carefully. For others, the context of this change is that the current order of lookup in trunk, in case there's no more token in URL path to process, is:
... so when routing a request to an object that has no view but The proposed code change as of this comment solves this by swapping this order. Even though technically backward incompatible, this reordering is effectively compatible because no valid object would have had index view and What this change fails to accomodate is that the code now pushes index view search so far down the priority list that now it's behind I think the additional change needed is to fold index view serving into a part of This way, index view can be served at the exact right priority, and code becomes cleaner. In addition, for typical facet implementations like Jelly, the script search can be front-loaded entirely to improve dispatching performance in case a class that doesn't define an index view. I'll make this follow up change and put it up as a new PR. |
|
Thanks mate I look forward to it |
The presence of index.html in a side file can be tested eagerly to avoid checking it at runtime. Since this feature is rarely used, this results in a performance gain. See: #97
|
Overtaken by #101 |
Currently the code performs index view lookup before doIndex handling. This behaviour is inconsistent with the relative ordering between xyz.jelly vs doXyz where the latter wins. This behaviour is considered sensible because it'd allow programmatic check before a view is rendered by using StaplerRequest.getView(). The code also has a performance improvement effect for those model objects that do not define any index view but doIndex, by going straight to doIndex without bothering to try index views. Objects that do define index view without doIndex will perform the same, because the presence of doIndex is tested during MetaClass.buildDispatchers(), and not during dispatching requests. See: #97
Allows the dispatchers to run first and avoids searching the classpath for index files for each facet.