-
Notifications
You must be signed in to change notification settings - Fork 7
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
Refactor: Performance tune for OASIS service #353
Conversation
…f QueryResult issue: #348
@@ -3,6 +3,7 @@ package connectivitymap | |||
import ( | |||
"sync" | |||
|
|||
"github.com/Telenav/osrm-backend/integration/service/oasis/internal/common" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still common? Aha
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will change it to domain/entity
:)
func (q *querier) NearByStationQuery(placeID common.PlaceID) []*common.RankedPlaceInfo { | ||
if results, ok := q.id2QueryResults[placeID]; ok { | ||
return results | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to remove else
block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Remove not necessary else
.
@@ -31,36 +31,33 @@ func NewStationGraph(currEnergyLevel, maxEnergyLevel float64, strategy chargings | |||
func (sg *stationGraph) setStartAndEndForGraph(currEnergyLevel, maxEnergyLevel float64) bool { | |||
startLocation := sg.querier.GetLocation(stationfindertype.OrigLocationID) | |||
if startLocation == nil { | |||
glog.Errorf("Failed to find %#v from Querier GetLocation()\n", stationfindertype.OrigLocationID) | |||
glog.Errorf("Failed to find %#v from Querier GetLocation()\n", stationfindertype.OrigLocationID.String()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use %s
instead, then you don't need to explicitly call String()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove .String()
Issue
Targeting issue: the issue will be CLOSED once the PR merged. If there is no issue that addresses the problem, please open a corresponding issue and link it here. Uses the Closes keyword to close them automatically.
Closes Profile OASIS service for charge station based routing - Round 1 #345, Closes Profile OASIS service for charge station based routing - Round 2 #349
Any other related issue? Mention them here.
Description
This pull improves performance for OASIS service for about 10 times. For more details please go to #345 and #349.
During performance tune and refactoring, I noticed there are some further work to be done:
common
, which is not following Telenav's behavior :)Tasklist
Lua
changesPre-requirements