Optimize /proc traversal #450

inercia · 2015-09-07T09:21:55Z

The objective of this PR is to reduce the number of syscalls when collecting info from /proc. This PR does that by:

reading the required files (/proc/*/[cmdline, comm, stat, fd/*], /proc/net/tcp*) once per cycle (spy.interval), parsing and caching the processes/connections information
keeping a cache of open files for /proc/*/[cmdline, comm, stat] so we do not open()/close() these files so often.
removing unnecessary stat()s when reading /proc directories

Changes:

the required code for reading/parsing /proc has been proved from procspy to /probe/proc
the Walker has been replaced by a proc.Reader that provides both Processes() and Connections(). Users can walk the list of processes/connections in the same way by providing a function to these functions. A CachingReader can be used for caching and getting this info, updated when Update(). The code for reading and parsing connection and processes has been simplified.
a CachingReader is initialized in /probe/main.go and used in both endpoint.Reporter and process.Reporter for reporting about processed and connections, and updated periodically by calling Update() every spy.interval seconds.
unit tests have been improved by mocking things like file system operations
things like Proc and Process have been unified, and many things from /probe/process have been moved to /probe/proc

This is part of the solution to #284

2opremio · 2015-09-07T17:06:04Z

@inercia I don't know if it provides all you need but maybe you could use (or even extend) https://github.com/mitchellh/go-ps

2opremio · 2015-09-07T17:13:11Z

Actually, I don't think go-ps provides connection/file information, but https://github.com/c9s/goprocinfo certainly does and it seems more mature.

EDIT: maybe it has the same problems as procspy, feel free to disregard my comment.

inercia · 2015-09-09T10:23:54Z

@2opremio thanks with the hint... goprocinfo seems interesting. The only problem I see is that they use Read()s instead of our current mechanism where we buf.ReadFrom(someFile) and the buf is obtained from a Pool, so we do not generate too much garbage... it's worth some investigation, though...

probe/proc/reader.go

+// returns some info like processes and connections
+type ProcReader interface {
+	// Processes walks through the processes
+	Processes(func(Process)) error


inercia · 2015-09-09T11:33:11Z

@tomwilkie or @peterbourgon could you please review this?

peterbourgon · 2015-09-09T12:45:23Z

@inercia Sure. Would you mind giving a short overview of what this PR does, and pointers to key types/functions/changes? (I know the basic context, just looking for your perspective.)

probe/proc/reader_linux.go

+	New: func() interface{} {
+		return bytes.NewBuffer(make([]byte, 0, 5000))
+	},
+}


inercia · 2015-09-10T20:05:59Z

@peterbourgon I have updated the PR description, so please let me know what you think about this...

inercia · 2015-09-11T07:56:16Z

This is the latest profile graph from this branch (using go 1.5). Compared with the original graph in #284, we can see that the number of syscalls has been vastly reduced, but we spend a lot of time in runtime.* now (too much garbage, maybe?). In particular, I think we are allocating too many small objects when generating reports (see the number of allocations, runtime.mallocgc, and garbage collections, runtime.scanobject)

peterbourgon · 2015-09-11T08:02:53Z

@inercia Thanks, that helps a lot. I'll give it a look.

probe/proc/net.go

+
+	c.LocalAddress = dupIP(c.LocalAddress)
+	c.RemoteAddress = dupIP(c.RemoteAddress)
+	return c


peterbourgon · 2015-09-11T15:49:42Z

The shape of it is good. Back to @inercia for those few comments (and rebasing, I guess).

…`probe`.

Code cleanups

…in the /proc dir

Some minor renames and cleanups.

peterbourgon · 2015-09-14T09:54:15Z

👍 pending Coveralls. (Maybe Coveralls is broken?)

inercia · 2015-09-14T12:28:43Z

@peterbourgon I think the coveralls hook has failed (the jobs is finisished, but the "Coverage pending" is still here). I'm merging....

Optimize /proc traversal

This reverts commit 46345e3, reversing changes made to bf3e9a1. Conflicts: probe/process/walker_darwin.go

inercia force-pushed the scope-284 branch 3 times, most recently from a2249b2 to 3fae0d7 Compare September 9, 2015 10:20

inercia reviewed Sep 9, 2015
View reviewed changes

probe/proc/reader.go

// returns some info like processes and connections

type ProcReader interface {

// Processes walks through the processes

Processes(func(Process)) error

This comment was marked as abuse.

Sign in to view

inercia force-pushed the scope-284 branch from 3fae0d7 to 52e8a6a Compare September 9, 2015 11:26

inercia changed the title ~~[WIP] Optimize /proc traversal~~ Optimize /proc traversal Sep 9, 2015

peterbourgon self-assigned this Sep 9, 2015

inercia mentioned this pull request Sep 10, 2015

Probe is using 40% cpu #284

Closed

inercia force-pushed the scope-284 branch 2 times, most recently from 654e5b0 to 62171ca Compare September 10, 2015 15:53

inercia reviewed Sep 10, 2015
View reviewed changes

probe/proc/reader_linux.go

New: func() interface{} {

return bytes.NewBuffer(make([]byte, 0, 5000))

},

}

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

peterbourgon reviewed Sep 11, 2015
View reviewed changes

probe/proc/net.go

c.LocalAddress = dupIP(c.LocalAddress)

c.RemoteAddress = dupIP(c.RemoteAddress)

return c

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

peterbourgon removed their assignment Sep 11, 2015

peterbourgon assigned inercia Sep 11, 2015

inercia added 4 commits September 14, 2015 10:44

Drop all the dependencies we had on procspy by moving some code to …

4331fb1

…`probe`.

Cache processes and connections when reading from the /proc

14dc6a5

Code cleanups

Keep a cache of open files, reducing the number of open/close cycles …

2f9f546

…in the /proc dir

Minor: refactored readerLinux/readIntoBuffer() for removing a parameter

026d8e9

inercia force-pushed the scope-284 branch from 8c90264 to 9a5406a Compare September 14, 2015 08:59

Use a mutex in the /proc files cache

a7dc0ac

Some minor renames and cleanups.

inercia force-pushed the scope-284 branch from 9a5406a to a7dc0ac Compare September 14, 2015 09:14

inercia assigned peterbourgon and unassigned inercia Sep 14, 2015

peterbourgon removed their assignment Sep 14, 2015

inercia added a commit that referenced this pull request Sep 14, 2015

Merge pull request #450 from weaveworks/scope-284

46345e3

Optimize /proc traversal

inercia merged commit 46345e3 into master Sep 14, 2015

peterbourgon deleted the scope-284 branch September 14, 2015 12:29

tomwilkie pushed a commit that referenced this pull request Sep 15, 2015

Revert "Merge pull request #450 from weaveworks/scope-284"

7d6b5a5

This reverts commit 46345e3, reversing changes made to bf3e9a1. Conflicts: probe/process/walker_darwin.go

inercia mentioned this pull request Sep 16, 2015

Optimize /proc traversal (again) #497

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize /proc traversal #450

Optimize /proc traversal #450

inercia commented Sep 7, 2015

2opremio commented Sep 7, 2015

2opremio commented Sep 7, 2015

inercia commented Sep 9, 2015

This comment was marked as abuse.

inercia commented Sep 9, 2015

peterbourgon commented Sep 9, 2015

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

inercia commented Sep 10, 2015

inercia commented Sep 11, 2015

peterbourgon commented Sep 11, 2015

This comment was marked as abuse.

This comment was marked as abuse.

peterbourgon commented Sep 11, 2015

peterbourgon commented Sep 14, 2015

inercia commented Sep 14, 2015

Optimize /proc traversal #450

Optimize /proc traversal #450

Conversation

inercia commented Sep 7, 2015

2opremio commented Sep 7, 2015

2opremio commented Sep 7, 2015

inercia commented Sep 9, 2015

This comment was marked as abuse.

inercia commented Sep 9, 2015

peterbourgon commented Sep 9, 2015

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

inercia commented Sep 10, 2015

inercia commented Sep 11, 2015

peterbourgon commented Sep 11, 2015

This comment was marked as abuse.

This comment was marked as abuse.

peterbourgon commented Sep 11, 2015

peterbourgon commented Sep 14, 2015

inercia commented Sep 14, 2015