Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove usage of GetMounts from GetCgroupMounts #496

Merged
merged 1 commit into from
Feb 4, 2016

Conversation

LK4D4
Copy link
Contributor

@LK4D4 LK4D4 commented Jan 21, 2016

GetMounts is very cpu-expensive. I'll change other funcs in this package
to reuse code from GetCgroupMounts later.

@crosbymichael
Copy link
Member

LGTM

1 similar comment
@dqminh
Copy link
Contributor

dqminh commented Jan 21, 2016

LGTM

for scanner.Scan() {
txt := scanner.Text()
fields := strings.Split(txt, " ")
if fields[len(fields)-3] != "cgroup" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use the post separator logic here as it will be more robust?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LK4D4 ^.^

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrunalp what do you mean? :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant that we can look for the separator -and then match the first word after instead of going minus 3 :) discretionary though :)

Sent from my iPhone

On Jan 30, 2016, at 4:43 AM, Alexander Morozov notifications@github.com wrote:

In libcontainer/cgroups/utils.go:

  •   if mount.Fstype == "cgroup" {
    

- m := Mount{Mountpoint: mount.Mountpoint, Root: mount.Root}

  •       for _, opt := range strings.Split(mount.VfsOpts, ",") {
    
  •           if strings.HasPrefix(opt, cgroupNamePrefix) {
    
  •               m.Subsystems = append(m.Subsystems, opt[len(cgroupNamePrefix):])
    
  •           }
    
  •           if allMap[opt] {
    
  •               m.Subsystems = append(m.Subsystems, opt)
    
  •           }
    
  • var res []Mount
  • scanner := bufio.NewScanner(f)
  • for scanner.Scan() {
  •   txt := scanner.Text()
    
  •   fields := strings.Split(txt, " ")
    
  •   if fields[len(fields)-3] != "cgroup" {
    
    @mrunalp what do you mean? :)


Reply to this email directly or view it on GitHub.

GetMounts is very cpu-expensive. I'll change other funcs in this package
to reuse code from GetCgroupMounts later.

Signed-off-by: Alexander Morozov <lk4d4@docker.com>
@LK4D4
Copy link
Contributor Author

LK4D4 commented Feb 1, 2016

@mrunalp I added optimization with "-" and also added test and benchmark.

@mrunalp
Copy link
Contributor

mrunalp commented Feb 2, 2016

Thanks. LGTM.

crosbymichael added a commit that referenced this pull request Feb 4, 2016
Remove usage of GetMounts from GetCgroupMounts
@crosbymichael crosbymichael merged commit 5fe15a5 into opencontainers:master Feb 4, 2016
@LK4D4 LK4D4 deleted the remove_sscanf branch February 10, 2016 21:29
stefanberger pushed a commit to stefanberger/runc that referenced this pull request Sep 8, 2017
Correction to User struct in specs-go/config.json
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants