Skip to content
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

Retire multiple definition of weight in oasis #348

Closed
CodeBear801 opened this issue May 18, 2020 · 3 comments
Closed

Retire multiple definition of weight in oasis #348

CodeBear801 opened this issue May 18, 2020 · 3 comments
Assignees
Labels
Prototype Proof of concept Refactor Rewrite existing code in order to improve its readability, reusability or structure
Milestone

Comments

@CodeBear801
Copy link

CodeBear801 commented May 18, 2020

subtask of #344

package connectivitymap

// Weight represent weight information
type Weight struct {
	Distance float64
	Duration float64
}

package stationfindertype

// Weight represent weight information
type Weight struct {
	Duration float64
	Distance float64
}

package connectivitymap

// QueryResult records topological query result
type QueryResult struct {
	StationID       string
	StationLocation *nav.Location
	Distance        float64
	Duration        float64
}

package spatialindexer

type RankedPlaceInfo struct {
	PlaceInfo
	Distance float64
	Duration float64
}

// PlaceInfo records place related information such as ID and location
type PlaceInfo struct {
	ID       PlaceID
	Location nav.Location
}

// PlaceID defines ID for given place(location, point of interest)
// Only the data used for pre-processing contains valid PlaceID
type PlaceID int64

package stationgraph

type edgeMetric struct {
	distance float64
	duration float64
}
@CodeBear801
Copy link
Author

CodeBear801 commented May 20, 2020

The definition in connectivitymap, spatialindexer and stationgraph is more related to handling users' request.

Here is the plan:

  • Keep structure of RankedPlaceInfo, use which to replace QueryResult
  • Only use PlaceID, remove StationID string
  • Create Weight struct in internal/common, every one will use the definition there
  • Change RankedPlaceInfo to
type RankedPlaceInfo struct {
	PlaceInfo
	Weight *common.Weight
}

// PlaceInfo records place related information such as ID and location
type PlaceInfo struct {
	ID       PlaceID
	Location *nav.Location
}

@CodeBear801
Copy link
Author

CodeBear801 commented May 20, 2020

Thinking about whether to record as Pointer or Value

// For definition like
type RankedPlaceInfo struct {
	PlaceInfo
	Weight *common.Weight
}

// PlaceInfo records place related information such as ID and location
type PlaceInfo struct {
	ID       PlaceID
	Location *nav.Location
}
// and for function return value

RankPlaceIDsByShortestDistance(center nav.Location, targets []*common.PlaceInfo) []*common.RankedPlaceInfo

Method Receiver

(a structA) or (a *structA)

From method's perspective, from Go's method receiver: Pointer vs Value

  • If not going to change and struct's definition is not huge, better pass by object
  • If going to change or object is huge, then pass by pointer

Member

But for struct members, I consider more about how to use them. For example,

place_1 connects: place_2, place_3, place_4, place_5...
place_2 connects: place_1, place_4, place_5, place_6...

And we might copy location data in the following process. Making a copy of a struct in Go

loc := nav.Location{...}
target := loc   // will copy basic types inside Location

loc := &nav.Location(...)
target := loc   // will copy pointer
                // struct's pointer is 32bits on 32bit machine, https://medium.com/@vCabbage/go-are-pointers-a-performance-optimization-a95840d3ef85

Consider that lots of places will hold place's location, from memory's perspective it would be better to hold a pointer. So do Weight, which is static data contains Distance, Duration, Price.

Return value

For the following case, whether to return a slice of value objects or pointers

RankPlaceIDsByShortestDistance(center nav.Location, targets []*common.PlaceInfo) []*common.RankedPlaceInfo

slice returns pointers, whether the slice is value objects or pointers depends on how we want to use them.

Others

Using pointer doesn't mean there is no cost, you could come here for information:Go: Are pointers a performance optimization?. Profiler will tell whether pointer or value object is better.

@CodeBear801
Copy link
Author

For simplicity, the implementation in 1328354 records all kind combination of locations in memory.

After this commit, memory increased from 3GB to 7.5GB, but performance go from 40 seconds to 10 seconds. More information could go to here: #349 (comment)

More memory optimization would go to next round.

CodeBear801 added a commit that referenced this issue May 21, 2020
@CodeBear801 CodeBear801 self-assigned this May 22, 2020
@CodeBear801 CodeBear801 added Prototype Proof of concept Refactor Rewrite existing code in order to improve its readability, reusability or structure labels May 22, 2020
@CodeBear801 CodeBear801 added this to the v10.4.0 milestone May 22, 2020
CodeBear801 added a commit that referenced this issue May 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Prototype Proof of concept Refactor Rewrite existing code in order to improve its readability, reusability or structure
Projects
None yet
Development

No branches or pull requests

1 participant