Speed up Version Checks #62216
Speed up Version Checks #62216
Conversation
The `fromId` method would show up in profiling and JIT analysis as not-inlinable because it's too large in the contexts it's used in in many cases and was consuming a surprising amount of cycles for computing the min compat versions. -> extract cold path from `fromId` to make JIT happy and cache minimumg compatible versions to fields.
original-brownbear
added
>non-issue
:Core/Infra/Core
|
Pinging @elastic/es-core-infra (:Core/Infra/Core) |
| @@ -285,6 +286,14 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws | |||
| static final List<Version> DECLARED_VERSIONS = Collections.unmodifiableList(getDeclaredVersions(Version.class)); | |||
| } | |||
|
|
|||
| // lazy initialized because we don't yet have the declared versions ready when instantiating the cached Version | |||
| // instances | |||
| private Version minCompatVersion; | |||
There was a problem hiding this comment.
technically shouldn't we declare these as volatile?
There was a problem hiding this comment.
technically shouldn't we declare these as volatile?
I figured no because we don't actively deduplicate this value so if we recompute it a few times (and realistically this is probably almost never going to happen) at startup it doesn't hurt but we pay the overhead in method size for volatile forever. But this was coded wrong even under that assumption now that I look at it again. I pushed 153763a so that it's safe even if out of order reads were to ever occur.
I tried this and it's painful to do that from the current code because if you want the same deduplicate this solution gives you, you'll have to be very careful about the order in which you initialize the fields. (and more importantly it adds new overhead for maintaining the class as well because currently we automatically get the min compatible versions right from the declared constants). Not sure it's worth it, the performance difference should be trivial but it looks like it makes things more complex. |
|
Thanks Jay! |
The `fromId` method would show up in profiling and JIT analysis as not-inlinable because it's too large in the contexts it's used in in many cases and was consuming a surprising amount of cycles for computing the min compat versions. -> extract cold path from `fromId` to make JIT happy and cache minimumg compatible versions to fields.
The `fromId` method would show up in profiling and JIT analysis as not-inlinable because it's too large in the contexts it's used in in many cases and was consuming a surprising amount of cycles for computing the min compat versions. -> extract cold path from `fromId` to make JIT happy and cache minimumg compatible versions to fields.
Found randomly this while looking into where we're using large amounts of code cache/meta-space for an SDH:
The
fromIdmethod would show up in profiling and JIT analysis as not-inlinable because it's too largein the contexts it's used in (deserialisation + transport header checks) in many cases and was consuming a surprising amount of cycles for computing the
min compat versions.
-> extract cold path from
fromIdto make JIT happy and cache minimum compatible versions to fields.