Re-evaluate performances for []*Struct over []Struct

[mimir-d]

Issue by marcoguerri Wednesday Feb 05, 2020 at 12:48 GMT Originally opened as https://github.com/facebookincubator/contest/issues/21

---

Using []Struct is in most cases probably desirable, but in some cases []*Struct might make sense (e.g. Targets). We should re-evaluate these cases. Here is where we use []*, excluding Targets:

pkg/test/fetcher.go:16: Fetch(interface{}) (string, []*TestStepDescriptor, error)
pkg/job/reporter.go:20: Report(cancel <-chan struct{}, parameters interface{}, results []*test.TestResult, ev testevent.Fetcher) (bool, interface{}, error)
pkg/job/job.go:27:      TestDescriptors []*test.TestDescriptor
pkg/job/job.go:63:      Tests       []*test.Test
pkg/runner/job_runner.go:49:            testResults []*test.TestResult
pkg/jobmanager/jobmanager.go:88:        tests := make([]*test.Test, 0, len(jd.TestDescriptors))
plugins/testfetchers/uri/uri.go:86:func (tf *URI) Fetch(params interface{}) (string, []*test.TestStepDescriptor, error) {
plugins/testfetchers/uri/uri.go:117:            Steps []*test.TestStepDescriptor
plugins/testfetchers/literal/literal.go:29:     Steps    []*test.TestStepDescriptor
plugins/testfetchers/literal/literal.go:54:func (tf *Literal) Fetch(params interface{}) (string, []*test.TestStepDescriptor, error) 

[mimir-d]

Comment by xaionaro Monday Mar 02, 2020 at 12:00 GMT

---

I guess we should pick the way is more handy/safe to use (instead of the more performant way). I believe a ConTest server will never be a highloaded server :)

So I think we just should have a discussion which way we consider more handy/safer and just add it to coding guidelines.

In my opinion using of pointers is (suddenly:) safer. For example:

for _, item := range mySlice {
    item.UpdateStatistics()
}

will work corrently only of mySlice is []*Item (not []Item). On the other hand we always need to keep in mind the case item == nil. But we could just add something like if item == nil { return } to methods like UpdateStatistics(). So my point is if we use pointers then we can use this items as just objects in any other language (which is intuitive), while we don't use pointers we should always be aware it's just a copy. So it seems to me convenient to use pointers for complex objects and to use non-pointers for basic types which does not have any internal properties we expect to change on calling a method (like []int instead of []*int).

If the performance is considered a priority then please let me know. But anyway I believe there's more important stuff to fix if the performance is important.