Remove Bytecode Compatibility Transformer

[basil]

Commit 47de54d introduced Bytecode Compatibility Transformer to allow core to do signature changes on fields that plugins might depend on. This functionality was never enabled for plugins, so there are no plugins with org.jenkinsci.bytecode.AdaptField annotations. The only org.jenkinsci.bytecode.AdaptField annotations are in core (I verified this with usage-in-plugins and searching for AdaptField in sources), and the vast majority of them have been deleted over the years. Only two fields with such annotations remain: hudson.model.Queue$Item#id and hudson.model.AbstractProject#triggers. Running usage-in-plugins against

hudson.model.Queue.Item#id
hudson.model.Queue$Item#id
hudson.model.AbstractProject#triggers

turned up exactly two references, both to hudson/model/Queue$Item#id:

• Slave Prerequisites, a plugin with 517 installs last released 9 years ago
• vertx, a plugin with 8 installs last released 9 years ago

Let's consider the cost of maintaining Bytecode Compatibility Transformer:

• Roughly ~2000 lines of non-trivial Java code has to be maintained.
• A dependency on a forked/repackaged version of ASM 6 has to be maintained. (Nothing else in the ecosystem uses this repackaged version of ASM 6, as I also verified with usage-in-plugins and by searching in sources.)
• There is no realistic test coverage in Jenkins core for this compatibility scenario, making maintenance harder.

Now let's consider the benefit of maintaining Bytecode Compatibility Transformer:

• Two plugins last released 9 years ago can access an integer hudson/model/Queue$Item#id field.

In my humble opinion, it is time to dispense with this subsystem.

Proposed changelog entries

Removed: The bytecode transformer that retains compatibility after changing public Java APIs and its associated hudson.ClassicPluginStrategy.noBytecodeTransformer system property have been removed.

Developer: Plugins that rely on the hudson.model.Queue$Item#id or hudson.model.AbstractProject#triggers fields, including Slave Prerequisites and vertx, must be updated to call the corresponding getters.

Proposed upgrade guidelines

Support for plugins that rely on the hudson.model.Queue$Item#id or hudson.model.AbstractProject#triggers fields, including Slave Prerequisites and vertx, has been dropped. Any such plugins must be removed prior to upgrading Jenkins. If you have customized the hudson.ClassicPluginStrategy.noBytecodeTransformer system property, you should remove this customization.

Maintainer checklist

Before the changes are marked as ready-for-merge:

• There are at least 2 approvals for the pull request and no outstanding requests for change
• Conversations in the pull request are over OR it is explicit that a reviewer does not block the change
• Changelog entries in the PR title and/or Proposed changelog entries are correct
• Proper changelog labels are set so that the changelog can be generated automatically
• If the change needs additional upgrade steps from users, upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the PR title. (example)
• If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

[daniel-beck]

For the specific fields it seems useful. I wonder about other fields in the future though. There's some stuff in Jenkins in particular I've recently seen that may benefit from this feature to make sure callers access methods rather than public fields.

OTOH if we'd never again use this feature due to the pain it's caused in the past, we can just remove it.

[basil]

OTOH if we'd never again use this feature due to the pain it's caused in the past, we can just remove it.

I do not think we would ever want to use this again. It is unnecessarily complex and opaque, lacks automated testing, and does not interoperate well with debuggers and other IDE tooling.

• Commit 47de54d wrote: "Let's see for a while until we open this functionality up to plugins." Well, it's been a while, and we still haven't opened this functionality up to plugins, so that is not a good sign.
• Judging from commit 710f168 this never worked all that well to begin with.
• Use of Bytecode Compatibility Transformer was considered and rejected in JEP 227.

I think it is time to move on.

[jglick]

Great! I would love to also get rid of @WithBridgeMethods but at least that runs at build time so you can see what is going on in bytecode (still awkward for IDEs).

[jglick]

I suspect this also improves Jenkins startup time when lots of plugins are enabled. The transformer has short-circuit logic to avoid processing a byte[] if it does not seem to be using the annotated fields. Unfortunately even this logic involves doing a full parse of the constant pool, meaning some nontrivial array scanning and object creation. If we wanted to keep this transformer, we would want it to take the list of annotated classes and fields in binary format, like (hudson/model/Queue$Item | hudson/model/AbstractProject) & (id | triggers), then do something fancy like the Commentz-Walter algorithm to avoid any more than trivial overhead on the majority of classes which do not match this first pass, before continuing to the constant pool scan and the next bypass.

[basil]

I suspect this also improves Jenkins startup time when lots of plugins are enabled.

I did some relatively unscientific tests with the set of plugins used at my company (133 plugins). After starting Jenkins once with this plugin set, I measured how long Jenkins took to get from cold startup to "Jenkins is fully up and running". Before this change, the results were 17.80s, 18.05s, 18.30s, 18.57s, and 19.23s (average 18.39s). After this change the results were 17.27s, 17.31s, 17.50s, 17.63s, and 18.29s (average 17.6s). So I guess I shaved off about a second on average. I'll take it!

[jglick]

Not sure if that is statistically significant or not, but by inspection this change reduces startup time by some amount. Thanks!

[timja]

This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

[oleg-nenashev]

No objections from me. I am not sure what is the impact on various 3rd party and proprietary plugin. In any case, BCT is very difficult to use and maintain. Let's take a risk. In the worst case we will be able to revert the dependency or create an API plugin for it if possible

[jglick]

create an API plugin for it

Does not make sense.