getPids - don't recursively traverse every dir in /proc

[cpuguy83]

What this PR does / why we need it:

filepath.Walk recursively traverses every dir, which is not what is
needed for getPids.
Instead only read the list of dirs in the top level of /proc.

benchmark              old ns/op     new ns/op     delta
BenchmarkGetPids-4     868684        195522        -77.49%

Speedup process lookup in /proc

[k8s-ci-robot]

@lanchongyizu: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/ok-to-test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[xiaoxubeii]

/ok-to-test

[lanchongyizu]

https://github.com/kubernetes/kubernetes/pull/66367/files#diff-815325f4d4776cc2282858482b77d5f5R134
Should be continue rather than return.

[cpuguy83]

Good catch.

[lanchongyizu]

https://github.com/kubernetes/kubernetes/pull/66367/files#diff-815325f4d4776cc2282858482b77d5f5R141
Same to the above.

[cpuguy83]

Updated, also optimized in case of many entries in /proc so that we don't allocate a huge slice.

[fcrisciani]

LGTM

[fcrisciani]

@xiaoxubeii PTAL

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fcrisciani]

still valid
/remove-lifecycle stale

[dims]

/test all
/assign @thockin

[thockin]

Would it be simpler to make a more surgical change? Something like:

    filepath.Walk("/proc", func(path string, info os.FileInfo, err error) error {
        if err != nil {
            // We should continue processing other directories/files
            return nil 
        }
        if path == "/proc" {
            // Skip the top-level dir, but recurse
            return nil 
        }
        fmt.Printf("path %v\n", path)
        if !info.IsDir() {
            // Skip files
            return nil 
        }
        // If the directory is not a number (i.e. not a PID), skip it
        base := filepath.Base(path)
        if _, err := strconv.Atoi(base); err != nil {
            fmt.Printf("skipping %v\n", path)
            return filepath.SkipDir
        }
        fmt.Printf("found pid %v => %v/cmdline\n", base, path)
        // ...process it...
        return filepath.SkipDir // done with this dir, don't recurse further
    })  

[thockin]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpuguy83, thockin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• pkg/OWNERS [thockin]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[thockin]

Did some quick tests, this is better and complete

[cpuguy83]

🎉 Thanks!