Skip to content

Avoid unnecessary FooService allocations #390

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.

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

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
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
4 changes: 1 addition & 3 deletions github/activity.go
Original file line number Diff line number Diff line change
@@ -9,9 +9,7 @@ package github
// methods of the GitHub API.
//
// GitHub API docs: http://developer.github.com/v3/activity/
type ActivityService struct {
client *Client
}
type ActivityService service

// FeedLink represents a link to a related resource.
type FeedLink struct {
4 changes: 1 addition & 3 deletions github/authorizations.go
Original file line number Diff line number Diff line change
@@ -47,9 +47,7 @@ const (
// an OAuth token.
//
// GitHub API docs: https://developer.github.com/v3/oauth_authorizations/
type AuthorizationsService struct {
client *Client
}
type AuthorizationsService service

// Authorization represents an individual GitHub authorization.
type Authorization struct {
4 changes: 1 addition & 3 deletions github/gists.go
Original file line number Diff line number Diff line change
@@ -14,9 +14,7 @@ import (
// methods of the GitHub API.
//
// GitHub API docs: http://developer.github.com/v3/gists/
type GistsService struct {
client *Client
}
type GistsService service

// Gist represents a GitHub's gist.
type Gist struct {
4 changes: 1 addition & 3 deletions github/git.go
Original file line number Diff line number Diff line change
@@ -9,6 +9,4 @@ package github
// methods of the GitHub API.
//
// GitHub API docs: http://developer.github.com/v3/git/
type GitService struct {
client *Client
}
type GitService service
35 changes: 21 additions & 14 deletions github/github.go
Original file line number Diff line number Diff line change
@@ -108,6 +108,8 @@ type Client struct {
rateLimits [categories]Rate // Rate limits for the client as determined by the most recent API calls.
mostRecent rateLimitCategory

common service

// Services used for talking to different parts of the GitHub API.
Activity *ActivityService
Authorizations *AuthorizationsService
@@ -125,6 +127,10 @@ type Client struct {
Reactions *ReactionsService
}

type service struct {
client *Client
}

// ListOptions specifies the optional parameters to various List methods that
// support pagination.
type ListOptions struct {
@@ -174,20 +180,21 @@ func NewClient(httpClient *http.Client) *Client {
uploadURL, _ := url.Parse(uploadBaseURL)

c := &Client{client: httpClient, BaseURL: baseURL, UserAgent: userAgent, UploadURL: uploadURL}
c.Activity = &ActivityService{client: c}
c.Authorizations = &AuthorizationsService{client: c}
c.Gists = &GistsService{client: c}
c.Git = &GitService{client: c}
c.Gitignores = &GitignoresService{client: c}
c.Issues = &IssuesService{client: c}
c.Organizations = &OrganizationsService{client: c}
c.PullRequests = &PullRequestsService{client: c}
c.Repositories = &RepositoriesService{client: c}
c.Search = &SearchService{client: c}
c.Users = &UsersService{client: c}
c.Licenses = &LicensesService{client: c}
c.Migrations = &MigrationService{client: c}
c.Reactions = &ReactionsService{client: c}
c.common.client = c
Copy link
Member

Choose a reason for hiding this comment

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

Since this optimization is quite small and subtle, I think it'd be nice to leave a small, unobtrusive comment here.

That would reduce the chances of someone accidentally reverting this optimization a year from now, because they may not immediately understand why the code is arranged in this way instead of something simpler.

Something like:

c.common.client = c // Reuse a single struct instead of allocating one for each service on the heap.

(Or something along those lines, phrased in a better way.)

Placing it inline, on the right, is a cue to only read the comment it if you care about editing this section of code.

It's up to you, if you think it's worth it.

Copy link
Member

Choose a reason for hiding this comment

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

As a side benefit, someone who cares about finding more details/context can use find this commit via git blame.

So the comment can be very short, and a more detailed explanation can go into the commit message (which will reference the issue #390).

c.Activity = (*ActivityService)(&c.common)
c.Authorizations = (*AuthorizationsService)(&c.common)
c.Gists = (*GistsService)(&c.common)
c.Git = (*GitService)(&c.common)
c.Gitignores = (*GitignoresService)(&c.common)
c.Issues = (*IssuesService)(&c.common)
c.Organizations = (*OrganizationsService)(&c.common)
c.PullRequests = (*PullRequestsService)(&c.common)
c.Repositories = (*RepositoriesService)(&c.common)
c.Search = (*SearchService)(&c.common)
c.Users = (*UsersService)(&c.common)
c.Licenses = (*LicensesService)(&c.common)
c.Migrations = (*MigrationService)(&c.common)
c.Reactions = (*ReactionsService)(&c.common)
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you go ahead and sort these as well? They were always supposed to be, but it looks like the last few got added to the bottom.

return c
}

4 changes: 1 addition & 3 deletions github/gitignore.go
Original file line number Diff line number Diff line change
@@ -11,9 +11,7 @@ import "fmt"
// GitHub API.
//
// GitHub API docs: http://developer.github.com/v3/gitignore/
type GitignoresService struct {
client *Client
}
type GitignoresService service

// Gitignore represents a .gitignore file as returned by the GitHub API.
type Gitignore struct {
4 changes: 1 addition & 3 deletions github/issues.go
Original file line number Diff line number Diff line change
@@ -14,9 +14,7 @@ import (
// methods of the GitHub API.
//
// GitHub API docs: http://developer.github.com/v3/issues/
type IssuesService struct {
client *Client
}
type IssuesService service

// Issue represents a GitHub issue on a repository.
type Issue struct {
4 changes: 1 addition & 3 deletions github/licenses.go
Original file line number Diff line number Diff line change
@@ -11,9 +11,7 @@ import "fmt"
// methods of the GitHub API.
//
// GitHub API docs: http://developer.github.com/v3/pulls/
type LicensesService struct {
client *Client
}
type LicensesService service

// License represents an open source license.
type License struct {
4 changes: 1 addition & 3 deletions github/migrations.go
Original file line number Diff line number Diff line change
@@ -16,9 +16,7 @@ import (
// in the GitHub API.
//
// GitHub API docs: https://developer.github.com/v3/migration/
type MigrationService struct {
client *Client
}
type MigrationService service

// Migration represents a GitHub migration (archival).
type Migration struct {
4 changes: 1 addition & 3 deletions github/orgs.go
Original file line number Diff line number Diff line change
@@ -14,9 +14,7 @@ import (
// in the GitHub API.
//
// GitHub API docs: http://developer.github.com/v3/orgs/
type OrganizationsService struct {
client *Client
}
type OrganizationsService service

// Organization represents a GitHub organization account.
type Organization struct {
4 changes: 1 addition & 3 deletions github/pulls.go
Original file line number Diff line number Diff line change
@@ -14,9 +14,7 @@ import (
// methods of the GitHub API.
//
// GitHub API docs: http://developer.github.com/v3/pulls/
type PullRequestsService struct {
client *Client
}
type PullRequestsService service

// PullRequest represents a GitHub pull request on a repository.
type PullRequest struct {
4 changes: 1 addition & 3 deletions github/reactions.go
Original file line number Diff line number Diff line change
@@ -11,9 +11,7 @@ import "fmt"
// GitHub API.
//
// GitHub API docs: https://developer.github.com/v3/reactions/
type ReactionsService struct {
client *Client
}
type ReactionsService service

// Reaction represents a GitHub reaction.
type Reaction struct {
4 changes: 1 addition & 3 deletions github/repos.go
Original file line number Diff line number Diff line change
@@ -11,9 +11,7 @@ import "fmt"
// methods of the GitHub API.
//
// GitHub API docs: http://developer.github.com/v3/repos/
type RepositoriesService struct {
client *Client
}
type RepositoriesService service

// Repository represents a GitHub repository.
type Repository struct {
4 changes: 1 addition & 3 deletions github/search.go
Original file line number Diff line number Diff line change
@@ -15,9 +15,7 @@ import (
// in the GitHub API.
//
// GitHub API docs: http://developer.github.com/v3/search/
type SearchService struct {
client *Client
}
type SearchService service

// SearchOptions specifies optional parameters to the SearchService methods.
type SearchOptions struct {
4 changes: 1 addition & 3 deletions github/users.go
Original file line number Diff line number Diff line change
@@ -11,9 +11,7 @@ import "fmt"
// methods of the GitHub API.
//
// GitHub API docs: http://developer.github.com/v3/users/
type UsersService struct {
client *Client
}
type UsersService service

// User represents a GitHub user.
type User struct {