perf: parallelize user directory scanning and XML unmarshaling at sta…

[rajat315315]

Description

This PR parallelizes user index initialization during startup inside hudson.model.User$AllUsers#scanAll.

Benefits

1. Accelerates Startup: Concurrently unmarshals XML configs and populates the registry cache, leveraging multi-core processors.
2. Thread-Safe Concurrent Cache: Utilizes the thread-safe ConcurrentHashMap (byName) for concurrent index registrations.
3. Resilient Failure Handling: Errors in individual user profiles are handled in-thread, logging issues without interrupting the overall indexing task.

Micro-benchmark Results (3,000 users, 10 iterations)

• Sequential Scan (Legacy): ~793.00 ms average per iteration.
• Parallel Scan (Optimized): ~265.10 ms average per iteration.
• Speedup: ~2.99x faster using Parallel Streams.

Fixes #27059

Testing done

I have added an automated test parallelScanAllWithMalformedConfig() that
that verifies the behavior of User.AllUsers.scanAll().

• Generates 10 valid user profiles on disk and saves them.
• Sets up two separate malformed config.xml files inside distinct directories:
  • One with incomplete tag nesting (syntax-invalid).
  • One mapping to a different class like Descriptor instead of User (semantics-invalid).
• Clears the registry memory cache.
• Invokes User.AllUsers.scanAll().
• Asserts that all 10 valid user registries are successfully loaded.
• Asserts that none of the malformed users abort the scan or are registered.
• Ensures thorough cleanup of directories on disk in a finally block.

Screenshots (UI changes only)

Before

After

Proposed changelog entries

parallelize user directory scanning and XML unmarshaling at startup

Proposed changelog category

/label major-rfe

Proposed upgrade guidelines

N/A

Submitter checklist

• The issue, if it exists, is well-described.
• The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples). Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
• There is automated testing or an explanation as to why this change has no tests.
• New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
• New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
• UI changes do not introduce regressions when enforcing the current default rules of Content Security Policy Plugin. In particular, new or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content Security Policy (CSP) directives (see documentation).
• For dependency updates, there are links to external changelogs and, if possible, full differentials.
• For new APIs and extension points, there is a link to at least one consumer.

Desired reviewers

@mention

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

Maintainer checklist

• There are at least two (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 is not blocking the change.
• Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
• Proper changelog labels are set so that the changelog can be generated automatically.
• If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
• If it would make sense to backport the change to LTS, be a Bug or Improvement, and either the issue or pull request must be labeled as lts-candidate to be considered.

[MarkEWaite]

Please complete the "testing done" section of the pull request template. The comment says:

Provide a clear description of how this change was tested. At minimum this should include proof that a computer has executed the changed lines. Ideally this should include an automated test or an explanation as to why this change has no tests. Note that automated test coverage is less than complete, so a successful PR build does not necessarily imply that a computer has executed the changed lines. If automated test coverage does not exist for the lines you are changing, you must describe the scenario(s) in which you manually tested the change. For frontend changes, include screenshots of the relevant page(s) before and after the change. For refactoring and code cleanup changes, exercise the code before and after the change and verify the behavior remains the same.

Please explain why you checked the box for "has automated tests" but only changed production code, without adding an automated test.

[Copilot]

Pull request overview

This pull request speeds up Jenkins startup user indexing by parallelizing the on-disk user directory scan and config.xml unmarshaling performed in hudson.model.User.AllUsers#scanAll.

Changes:

• Switch user directory iteration from a sequential for loop to a parallel stream.
• Preserve existing per-user failure handling behavior (skip invalid directories / missing or unreadable config.xml / invalid IDs) while allowing other users to load concurrently.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[MarkEWaite]

Thanks for the pull request.

Please complete the "testing done" section of the pull request description. Include a description of the interactive testing you performed to assure that the change is ready for large scale use.

Please provide one or more automated tests that address the concerns raised in the GitHub Copilot review of the pull request.

[MarkEWaite]

I created an installation with over 40000 users with the following script in the Groovy script console:

import hudson.security.HudsonPrivateSecurityRealm
import jenkins.model.Jenkins

def jenkins = Jenkins.getInstance()
def securityRealm = jenkins.getSecurityRealm()

if (securityRealm instanceof HudsonPrivateSecurityRealm) {
    (1..42317).each { i ->
        def userId = String.format("user_%05d", i)
        def password = "InsecurePassword2026_${i}"
        def fullName = "Test User ${i}"

        // Check if the user already exists to prevent throwing errors
        def existingUser = securityRealm.getUser(userId)

        if (existingUser == null) {
            try {
                // Create the account in the HudsonPrivateSecurityRealm
                def newUser = securityRealm.createAccount(userId, password)

                // Set metadata properties
                newUser.setFullName(fullName)

                if (i % 50 == 0) {
                    println "Successfully created user: ${userId}"
                }
            } catch (Exception e) {
                println "Failed to create user ${userId}: ${e.message}"
            }
        } else {
            println "User ${userId} already exists. Skipping."
        }
    }

    // Save configuration to disk
    jenkins.save()

}

I saw no issues starting with that many users. The "Manage Jenkins" / "Users" page is slow to render because it does not use pagination to limit the number of rows shown to the user, but that is a pre-existing condition that is not changed by this pull request.

[MarkEWaite]

Build fails on ci.jenkins.io and will fail for you locally if you run mvn -DskipTests clean verify. Please fix the build failure. Maintainers should not spend time reviewing pull requests that fail to build.

[rajat315315]

I am sorry @MarkEWaite to ask this, but can I contribute another PR?
Or should I wait till this gets merged?

My motivation is to make Jenkins run super smooth and resolve all bottlenecks.

[MarkEWaite]

I am sorry @MarkEWaite to ask this, but can I contribute another PR?
Or should I wait till this gets merged?

Wait until this is merged.

My motivation is to make Jenkins run super smooth and resolve all bottlenecks.

That's a great motivation, but one of the most important bottlenecks is to learn the patterns of working with this repository. As an example, you haven't documented large scale testing that you've done, similar to the testing that I did. I asked for it earlier when I wrote:

Please complete the "testing done" section of the pull request description. Include a description of the interactive testing you performed to assure that the change is ready for large scale use.

[daniel-beck]

@MarkEWaite MarkEWaite added the needs-security-review label 3 days ago

We're a little busy at the moment, so do not expect this review any time soon. FWIW I don't see anything obviously security-critical here.

[MarkEWaite]

When this pull request is included in a Jenkins build in the Jenkins acceptance test harness, it fails tests as noted in pull request:

• jenkinsci/acceptance-test-harness#2800

Those test failures must be resolved before this can be merged.

When this pull request is run in a Jenkins build in the Jenkins plugin Bill of Materials, it fails tests as noted in pull request:

• jenkinsci/bom#7068

Those test failures must be resolved before this can be merged.

[rajat315315]

Let me fix them.

[rajat315315]

Acceptance Test Harness (ATH) Verification
I cloned the acceptance-test-harness repository and analyzed the CI test check runs for the PR. The failures in split5 (which ran AntPluginTest) as well as other split failures (JobDslPluginTest, JavadocPluginTest, GradlePluginTest, JUnitPluginTest) were caused by the unmarshaling of job builders/publishers failing during Jenkins startup due to the TCCL bug.

To verify my fix, I ran the tests locally against my compiled Jenkins war:

Javadoc Tests:

PATH=/home/rajat/maven/apache-maven-3.9.9/bin:$PATH JENKINS_WAR=/home/rajat/jenkins/war/target/jenkins.war BROWSER=firefox mvn test -Dtest=JavadocPluginTest

Result: BUILD SUCCESS (3/3 tests passed).

[rajat315315]

I will keep in mind to run Acceptance Test Harness (ATH) Verification from next time onwards.

[rajat315315]

@MarkEWaite Could you brief about the merging process? Like will it take time to merge?
Anything is pending from my side?

[MarkEWaite]

@MarkEWaite Could you brief about the merging process? Like will it take time to merge? Anything is pending from my side?

The pull request must be reviewed by the security team before it can be merged. They are very busy and don't know when they will be able to review it. I'm unwilling to merge it until they've reviewed it.

It has passed the acceptance test harness and the plugin compatibility tester.