Skip to content

Don't search/list every single repo reference when explicit references are provided #112

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Don't search/list every single repo reference when explicit references are provided #112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9d71973
Don't search/list every single repo reference when explicit reference…
elhmn Sep 25, 2023
721e353
Still run `git for-each-ref` when explicit roots and reference filter…
elhmn Sep 29, 2023
0315cba
Don't print the referecence count when explicit roots are provided
elhmn Sep 29, 2023
d407c86
Don't print reference count for json v1
elhmn Oct 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 31 additions & 15 deletions git-sizer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,22 +306,34 @@ func mainImplementation(ctx context.Context, stdout, stderr io.Writer, args []st
progressMeter = meter.NewProgressMeter(stderr, 100*time.Millisecond)
}

refRoots, err := sizes.CollectReferences(ctx, repo, rg)
if err != nil {
return fmt.Errorf("determining which reference to scan: %w", err)
}

roots := make([]sizes.Root, 0, len(refRoots)+len(flags.Args()))
for _, refRoot := range refRoots {
roots = append(roots, refRoot)
var roots []sizes.Root
var explicitRoots []sizes.Root
// If arguments are provided, use them as explicit roots.
if len(flags.Args()) > 0 {
explicitRoots = make([]sizes.Root, 0, len(flags.Args()))
for _, arg := range flags.Args() {
oid, err := repo.ResolveObject(arg)
if err != nil {
return fmt.Errorf("resolving command-line argument %q: %w", arg, err)
}
explicitRoots = append(explicitRoots, sizes.NewExplicitRoot(arg, oid))
}
}

for _, arg := range flags.Args() {
oid, err := repo.ResolveObject(arg)
// If no reference filters and no explicit roots were provided
if git.IsNoReferencesFilter(rgb.GetTopLevelGroup().GetFilter()) {
Copy link
Member

Choose a reason for hiding this comment

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

Instead of revealing internal details via both RefGroupBuilder.GetTopLevelGroup() and refGroup.GetFilter() plus needing the strange function IsNoReferencesFilter(), how about adding a special-purpose method to RefGroupBuilder, something like the following:

// RefoptsSeen returns `true` iff any reference selection options were
// used. It is only valid after `Finish()` has been called.
func (rgb *RefGroupBuilder) RefoptsSeen() bool {
	return rgb.topLevelGroup.filter != git.NoReferencesFilter
}

git.NoReferencesFilter is a singleton, so I think that a simple comparison like this should suffice.

roots = explicitRoots
} else {
refRoots, err := sizes.CollectReferences(ctx, repo, rg)
if err != nil {
return fmt.Errorf("resolving command-line argument %q: %w", arg, err)
return fmt.Errorf("determining which reference to scan: %w", err)
}

roots = make([]sizes.Root, 0, len(refRoots)+len(explicitRoots))
for _, refRoot := range refRoots {
roots = append(roots, refRoot)
}
roots = append(roots, sizes.NewExplicitRoot(arg, oid))
roots = append(roots, explicitRoots...)
}

historySize, err := sizes.ScanRepositoryUsingGraph(
Expand All @@ -331,14 +343,18 @@ func mainImplementation(ctx context.Context, stdout, stderr io.Writer, args []st
return fmt.Errorf("error scanning repository: %w", err)
}

formatOptions := sizes.FormatOptions{
WithoutReferenceCount: git.IsNoReferencesFilter(rgb.GetTopLevelGroup().GetFilter()),
}

if jsonOutput {
var j []byte
var err error
switch jsonVersion {
case 1:
j, err = json.MarshalIndent(historySize, "", " ")
j, err = json.MarshalIndent(historySize.JsonV1Format(&formatOptions), "", " ")
case 2:
j, err = historySize.JSON(rg.Groups(), threshold, nameStyle)
j, err = historySize.JSON(rg.Groups(), threshold, nameStyle, formatOptions)
default:
return fmt.Errorf("JSON version must be 1 or 2")
}
Expand All @@ -348,7 +364,7 @@ func mainImplementation(ctx context.Context, stdout, stderr io.Writer, args []st
fmt.Fprintf(stdout, "%s\n", j)
} else {
if _, err := io.WriteString(
stdout, historySize.TableString(rg.Groups(), threshold, nameStyle),
stdout, historySize.TableString(rg.Groups(), threshold, nameStyle, formatOptions),
); err != nil {
return fmt.Errorf("writing output: %w", err)
}
Expand Down
5 changes: 5 additions & 0 deletions git/ref_filter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,8 @@ type regexpFilter struct {
func (f regexpFilter) Filter(refname string) bool {
return f.re.MatchString(refname)
}

func IsNoReferencesFilter(val interface{}) bool {
_, ok := val.(noReferencesFilter)
return ok
}
Comment on lines +143 to +147
Copy link
Member

Choose a reason for hiding this comment

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

RefGroupBuilder.RefoptsSeen() could make this could go away.

4 changes: 4 additions & 0 deletions internal/refopts/ref_group.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ type refGroup struct {
otherRefGroup *sizes.RefGroup
}

func (rg *refGroup) GetFilter() git.ReferenceFilter {
return rg.filter
}

Comment on lines +33 to +36
Copy link
Member

Choose a reason for hiding this comment

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

RefGroupBuilder.RefoptsSeen() could make this could go away.

func (rg *refGroup) collectSymbols(refname string) (bool, []sizes.RefGroupSymbol) {
walk := false
var symbols []sizes.RefGroupSymbol
Expand Down
4 changes: 4 additions & 0 deletions internal/refopts/ref_group_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ type RefGroupBuilder struct {
groups map[sizes.RefGroupSymbol]*refGroup
}

func (rgb *RefGroupBuilder) GetTopLevelGroup() *refGroup {
return rgb.topLevelGroup
}

Comment on lines +25 to +28
Copy link
Member

Choose a reason for hiding this comment

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

RefGroupBuilder.RefoptsSeen() could make this could go away.

// NewRefGroupBuilder creates and returns a `RefGroupBuilder`
// instance.
func NewRefGroupBuilder(configger Configger) (*RefGroupBuilder, error) {
Expand Down
76 changes: 49 additions & 27 deletions sizes/output.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,19 @@ func (s TagSize) String() string {
return fmt.Sprintf("tag_depth=%d", s.TagDepth)
}

func (s *HistorySize) JsonV1Format(opts *FormatOptions) *HistorySize {
if opts == nil {
return s
}

if opts.WithoutReferenceCount {
s.ReferenceCount = 0
s.ReferenceGroups = nil
}

return s
}

func (s *HistorySize) String() string {
return fmt.Sprintf(
"unique_commit_count=%d, unique_commit_count = %d, max_commit_size = %d, "+
Expand Down Expand Up @@ -86,22 +99,22 @@ type section struct {
contents []tableContents
}

func newSection(name string, contents ...tableContents) *section {
return &section{
func newSection(name string, contents ...tableContents) section {
return section{
name: name,
contents: contents,
}
}

func (s *section) Emit(t *table) {
func (s section) Emit(t *table) {
for _, c := range s.contents {
subTable := t.subTable(s.name)
c.Emit(subTable)
t.addSection(subTable)
}
}

func (s *section) CollectItems(items map[string]*item) {
func (s section) CollectItems(items map[string]*item) {
for _, c := range s.contents {
c.CollectItems(items)
}
Expand Down Expand Up @@ -141,7 +154,7 @@ func newItem(
}
}

func (i *item) Emit(t *table) {
func (i item) Emit(t *table) {
levelOfConcern, interesting := i.levelOfConcern(t.threshold)
if !interesting {
return
Expand All @@ -154,7 +167,7 @@ func (i *item) Emit(t *table) {
)
}

func (i *item) Footnote(nameStyle NameStyle) string {
func (i item) Footnote(nameStyle NameStyle) string {
if i.path == nil || i.path.OID == git.NullOID {
return ""
}
Expand All @@ -173,7 +186,7 @@ func (i *item) Footnote(nameStyle NameStyle) string {
// If this item's alert level is at least as high as the threshold,
// return the string that should be used as its "level of concern" and
// `true`; otherwise, return `"", false`.
func (i *item) levelOfConcern(threshold Threshold) (string, bool) {
func (i item) levelOfConcern(threshold Threshold) (string, bool) {
value, overflow := i.value.ToUint64()
if overflow {
return "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!", true
Expand All @@ -188,11 +201,11 @@ func (i *item) levelOfConcern(threshold Threshold) (string, bool) {
return stars[:int(alert)], true
}

func (i *item) CollectItems(items map[string]*item) {
items[i.symbol] = i
func (i item) CollectItems(items map[string]*item) {
items[i.symbol] = &i
}

func (i *item) MarshalJSON() ([]byte, error) {
func (i item) MarshalJSON() ([]byte, error) {
// How we want to emit an item as JSON.
value, _ := i.value.ToUint64()

Expand Down Expand Up @@ -224,7 +237,7 @@ func (i *item) MarshalJSON() ([]byte, error) {

// Indented returns an `item` that is just like `i`, but indented by
// `depth` more levels.
func (i *item) Indented(depth int) tableContents {
func (i item) Indented(depth int) tableContents {
return &indentedItem{
tableContents: i,
depth: depth,
Expand All @@ -236,7 +249,7 @@ type indentedItem struct {
depth int
}

func (i *indentedItem) Emit(t *table) {
func (i indentedItem) Emit(t *table) {
subTable := t.indented("", i.depth)
i.tableContents.Emit(subTable)
t.addSection(subTable)
Expand Down Expand Up @@ -373,8 +386,9 @@ type table struct {

func (s *HistorySize) TableString(
refGroups []RefGroup, threshold Threshold, nameStyle NameStyle,
opts FormatOptions,
) string {
contents := s.contents(refGroups)
contents := s.contents(refGroups, opts)
t := table{
threshold: threshold,
nameStyle: nameStyle,
Expand Down Expand Up @@ -454,17 +468,20 @@ func (t *table) formatRow(
)
}

type FormatOptions struct {
WithoutReferenceCount bool
}

func (s *HistorySize) JSON(
refGroups []RefGroup, threshold Threshold, nameStyle NameStyle,
) ([]byte, error) {
contents := s.contents(refGroups)
refGroups []RefGroup, threshold Threshold, nameStyle NameStyle, opts FormatOptions) ([]byte, error) {
contents := s.contents(refGroups, opts)
items := make(map[string]*item)
contents.CollectItems(items)
j, err := json.MarshalIndent(items, "", " ")
return j, err
}

func (s *HistorySize) contents(refGroups []RefGroup) tableContents {
func (s *HistorySize) contents(refGroups []RefGroup, opts FormatOptions) tableContents {
S := newSection
I := newItem
metric := counts.Metric
Expand All @@ -489,6 +506,20 @@ func (s *HistorySize) contents(refGroups []RefGroup) tableContents {
rgis = append(rgis, rgi.Indented(indent))
}

var refCountSection section
if !opts.WithoutReferenceCount {
refCountSection = S(
"References",
I("referenceCount", "Count",
"The total number of references",
nil, s.ReferenceCount, metric, "", 25e3),
S(
"",
rgis...,
),
)
}

Comment on lines +509 to +522
Copy link
Member

Choose a reason for hiding this comment

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

This approach seems to depend on the fact that even though the uninitialized section is inserted into the table, it doesn't output anything. I think that's correct ✔️

return S(
"",
S(
Expand Down Expand Up @@ -533,16 +564,7 @@ func (s *HistorySize) contents(refGroups []RefGroup) tableContents {
nil, s.UniqueTagCount, metric, "", 25e3),
),

S(
"References",
I("referenceCount", "Count",
"The total number of references",
nil, s.ReferenceCount, metric, "", 25e3),
S(
"",
rgis...,
),
),
refCountSection,
),

S("Biggest objects",
Expand Down
4 changes: 2 additions & 2 deletions sizes/sizes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,11 +158,11 @@ type HistorySize struct {
// The number of references analyzed. Note that we don't eliminate
// duplicates if the user passes the same reference more than
// once.
ReferenceCount counts.Count32 `json:"reference_count"`
ReferenceCount counts.Count32 `json:"reference_count,omitempty"`

// ReferenceGroups keeps track of how many references in each
// reference group were scanned.
ReferenceGroups map[RefGroupSymbol]*counts.Count32 `json:"reference_groups"`
ReferenceGroups map[RefGroupSymbol]*counts.Count32 `json:"reference_groups,omitempty"`

// The maximum TreeSize in the analyzed history (where each
// attribute is maximized separately).
Expand Down