Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

feat/internal/memcmd: add internal/memcmd package to allow for memory tracking of exec.Cmd processes#62803

Merged
ggilmore merged 3 commits into
mainfrom
05-20-wip_keep_expriementing_with_linux_memory_observations
Jun 10, 2024
Merged

feat/internal/memcmd: add internal/memcmd package to allow for memory tracking of exec.Cmd processes#62803
ggilmore merged 3 commits into
mainfrom
05-20-wip_keep_expriementing_with_linux_memory_observations

Conversation

@ggilmore

@ggilmore ggilmore commented May 20, 2024

Copy link
Copy Markdown
Contributor

This PR adds a new package memcmd, that adds a new abstraction called "Observer" that allows you to track the memory that a command (and all of its children) is using. (This package uses a polling approach with procfs, since maxRSS on Linux is otherwise unreliable for our purposes).

Example usage

import (
	"context"
	"fmt"
	"os/exec"
	"time"

	"github.com/sourcegraph/sourcegraph/internal/memcmd"
)

func Example() {
	const template = `
#!/usr/bin/env bash
set -euo pipefail

word=$(head -c "$((10 * 1024 * 1024))" </dev/zero | tr '\0' '\141') # 10MB worth of 'a's
sleep 1
echo ${#word}
`

	cmd := exec.Command("bash", "-c", template)
	err := cmd.Start()
	if err != nil {
		panic(err)
	}

	observer, err := memcmd.NewLinuxObserver(context.Background(), cmd, 1*time.Millisecond)
	if err != nil {
		panic(err)
	}

	observer.Start()
	defer observer.Stop()

	err = cmd.Wait()
	if err != nil {
		panic(err)
	}

	memoryUsage, err := observer.MaxMemoryUsage()
	if err != nil {
		panic(err)
	}

	fmt.Println((0 < memoryUsage && memoryUsage < 50*1024*1024)) // Output should be between 0 and 50MB

	// Output:
	// true
}

Test plan

Unit tests

Note that some tests only work on darwin, so you'll have to run those locally.

Changelog

This feature adds a package that allows us to track the memory usage of commands invoked via exec.Cmd.

@cla-bot cla-bot Bot added the cla-signed label May 20, 2024

ggilmore commented May 20, 2024

Copy link
Copy Markdown
Contributor Author

@github-actions github-actions Bot added team/product-platform team/source Tickets under the purview of Source - the one Source to graph it all labels May 20, 2024
@ggilmore ggilmore force-pushed the 05-20-wip_keep_expriementing_with_linux_memory_observations branch 2 times, most recently from 454ea2d to 3200fec Compare May 29, 2024 22:23
@ggilmore ggilmore force-pushed the 05-20-wip_keep_expriementing_with_linux_memory_observations branch 7 times, most recently from 6e01906 to 5477d17 Compare June 5, 2024 23:25
@ggilmore ggilmore force-pushed the 05-20-wip_keep_expriementing_with_linux_memory_observations branch 2 times, most recently from 8c7c2c8 to fa9f739 Compare June 7, 2024 08:14
@ggilmore ggilmore changed the title wip: keep expriementing with linux memory observations add internal/memcmd package to allow for memory observation of Linux Processes Jun 7, 2024
@ggilmore ggilmore force-pushed the 05-20-wip_keep_expriementing_with_linux_memory_observations branch 2 times, most recently from f34c905 to 7836694 Compare June 7, 2024 08:20
@ggilmore ggilmore changed the title add internal/memcmd package to allow for memory observation of Linux Processes add internal/memcmd package to allow for memory tracking of exec.Cmd processes Jun 7, 2024
@ggilmore ggilmore force-pushed the 05-20-wip_keep_expriementing_with_linux_memory_observations branch from 7836694 to 3e3654d Compare June 7, 2024 08:22
Comment thread internal/memcmd/BUILD.bazel
@ggilmore ggilmore force-pushed the 05-20-wip_keep_expriementing_with_linux_memory_observations branch 5 times, most recently from c5f3b77 to 182220b Compare June 8, 2024 01:42
@ggilmore ggilmore marked this pull request as ready for review June 8, 2024 01:52
@ggilmore ggilmore requested a review from a team June 8, 2024 01:53

@eseliger eseliger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM pending the bazel question from Noah and an approving review from the dev-infra team who own bazel

Comment thread internal/memcmd/observer.go
Comment thread internal/memcmd/observer_darwin.go Outdated
Comment thread internal/memcmd/observer_darwin_test.go Outdated
Comment thread internal/memcmd/observer_darwin_test.go Outdated
Comment thread internal/memcmd/observer_test.go Outdated
@graphite-app

graphite-app Bot commented Jun 10, 2024

Copy link
Copy Markdown

Photo gif. A hand giving a thumbs-up appears in front of a photo of Harrison Ford. (Added via Giphy)

Comment thread internal/memcmd/observer.go Outdated
Comment thread internal/memcmd/observer.go
@ggilmore ggilmore force-pushed the 05-20-wip_keep_expriementing_with_linux_memory_observations branch from 182220b to 5f49f48 Compare June 10, 2024 18:59
@ggilmore ggilmore force-pushed the 05-20-wip_keep_expriementing_with_linux_memory_observations branch 4 times, most recently from 456f90e to a223c9a Compare June 10, 2024 20:13
@ggilmore ggilmore requested review from a team and Strum355 June 10, 2024 20:17
Co-authored-by: Noah Santschi-Cooney <noah@santschi-cooney.ch>
@ggilmore ggilmore force-pushed the 05-20-wip_keep_expriementing_with_linux_memory_observations branch from a223c9a to 9dd391b Compare June 10, 2024 20:54
@ggilmore ggilmore force-pushed the 05-20-wip_keep_expriementing_with_linux_memory_observations branch from 2044667 to ea401d0 Compare June 10, 2024 21:10
@ggilmore ggilmore changed the title add internal/memcmd package to allow for memory tracking of exec.Cmd processes feat/internal/memcmd: add internal/memcmd package to allow for memory tracking of exec.Cmd processes Jun 10, 2024
@ggilmore ggilmore merged commit aa1121c into main Jun 10, 2024
@ggilmore ggilmore deleted the 05-20-wip_keep_expriementing_with_linux_memory_observations branch June 10, 2024 21:20

Copy link
Copy Markdown
Contributor Author

Merge activity

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed team/product-platform team/source Tickets under the purview of Source - the one Source to graph it all

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants