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

[FEATURE REQUEST] Improve perfomance #19

Open
LdDl opened this issue Apr 24, 2021 · 3 comments
Open

[FEATURE REQUEST] Improve perfomance #19

LdDl opened this issue Apr 24, 2021 · 3 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested

Comments

@LdDl
Copy link
Owner

LdDl commented Apr 24, 2021

Is your feature request related to a problem? Please describe.
Let's take a laptop:

goos: windows
goarch: amd64
pkg: github.com/LdDl/ch
cpu: Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz

Run test (with debugged flag ON) on it for this Graph file containing ~211k edges:

go test -timeout 30s -v -run ^TestShortestPath$ github.com/LdDl/ch
....
--- PASS: TestShortestPath (10.32s)
...

Bassicaly it has taken ~10seconds to preprocess graph. It's not that bad. But.

Let's take another Graph file containing 251k edges:

go test -timeout 5000s -v -run ^TestShortestPath$ github.com/LdDl/ch
....
--- PASS: TestShortestPath (4240.13s)
...

Now it's about 70+ minutes. I guess the problem with that graph is (a) topology + (b) a lot of Dijkstra function calls during contraction process on last ~10k steps in priority queue (vertices sorted by importance).

Something needs to be done with preparing contraction hierarchies: improve heuristics (I don't think that ~[edges_num*2] number of generated shortcuts is a good ...) / delete unnecessary calculations / refactor code (probably find hidden bugs)

Additional context
Used heuristics currently:

  1. Edge difference:
// computeImportance Update importance of vertex
func (vertex *Vertex) computeImportance() {
	// Worst possible shortcuts number throught the vertex is: NumWorstShortcuts = NumIncomingEdges*NumOutcomingEdges
	vertex.shortcutCover = len(vertex.inIncidentEdges) * len(vertex.outIncidentEdges)
	// Number of total incident edges is: NumIncomingEdges+NumOutcomingEdges
	vertex.incidentEdgesNum = len(vertex.inIncidentEdges) + len(vertex.outIncidentEdges)
	// Edge difference is between NumWorstShortcuts and TotalIncidentEdgesNum
	vertex.edgeDiff = vertex.shortcutCover - vertex.incidentEdgesNum
	// [+] Spatial diversity heuristic: for each vertex maintain a count of the number of neighbors that have already been contracted [vertex.delNeighbors], and add this to the summary importance
	// note: the more neighbours have already been contracted, the later this vertex will be contracted in further.
	// [+] Bidirection edges heuristic: for each vertex check how many bidirected incident edges vertex has.
	// note: the more bidirected incident edges == less important vertex is.
	vertex.importance = vertex.edgeDiff*14 + vertex.incidentEdgesNum*25 + vertex.delNeighbors*10 - vertex.bidirectedEdges()
}

2 .Lazy update heuristic:

// update Importance of vertex "on demand" as follows:
// Before contracting vertex with currently smallest Importance, recompute its Importance and see if it is still the smallest
// If not pick next smallest one, recompute its Importance and see if that is the smallest now; If not, continue in same way
vertex := heap.Pop(graph.pqImportance).(*Vertex)
vertex.computeImportance()
if graph.pqImportance.Len() != 0 && vertex.importance > graph.pqImportance.Peek().importance {
    graph.pqImportance.Push(vertex)
    continue
}
@LdDl LdDl added enhancement New feature or request help wanted Extra attention is needed question Further information is requested labels Apr 24, 2021
@LdDl LdDl self-assigned this Apr 24, 2021
@LdDl
Copy link
Owner Author

LdDl commented May 13, 2021

Trying to do parallelism (not working for currently - WIP): https://github.com/LdDl/ch/tree/optional-parallelism

go.exe test -count=1 -v -timeout 30s -run ^TestParallelShortestPath$ github.com/LdDl/ch

@LdDl
Copy link
Owner Author

LdDl commented Aug 19, 2021

Trying to do parallelism (not working for currently - WIP): https://github.com/LdDl/ch/tree/optional-parallelism

go.exe test -count=1 -v -timeout 30s -run ^TestParallelShortestPath$ github.com/LdDl/ch

Result is right sometimes, but it is inconsistent still:

Contraction Order: 187853 / 187853, Remain vertices in heap: 0. Currect shortcuts num: 390850

When for sure it should be

Contraction Order: 187852 / 187853, Remain vertices in heap: 0. Currect shortcuts num: 395295

@LdDl
Copy link
Owner Author

LdDl commented Aug 20, 2021

Oh well
The problem is in graph.insertShortcuts(batchShortcuts) calls.
They could not be parallelized since they are inserting incident edges and thus leads us to wrong dijkstra() calls (and this is why the results are inconsistent)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

1 participant