Skip to content

[hail] use val for IndexedRVDSpec partitioner#6629

Merged
danking merged 2 commits intohail-is:masterfrom
chrisvittal:indexedrvdspec-val-partitioner
Jul 12, 2019
Merged

[hail] use val for IndexedRVDSpec partitioner#6629
danking merged 2 commits intohail-is:masterfrom
chrisvittal:indexedrvdspec-val-partitioner

Conversation

@chrisvittal
Copy link
Collaborator

When using indexed reads, we were creating a new partitioner for every
partiton in tmpPartitioner. It was not good.

When using indexed reads, we were creating a new partitioner for every
partiton in tmpPartitioner. It was not good.
@tpoterba
Copy link
Contributor

oof, this is bad enough we might want to do yet another release

Copy link
Contributor

@tpoterba tpoterba left a comment

Choose a reason for hiding this comment

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

add a benchmark that will catch this performance problem

@chrisvittal
Copy link
Collaborator Author

You have no idea. I went from waiting over an hour for a stage to begin to fast enough.

@chrisvittal
Copy link
Collaborator Author

This uses an undocumented feature. It can wait for a little while.

@tpoterba
Copy link
Contributor

oh, phew, it's only on readIndexed

@chrisvittal
Copy link
Collaborator Author

Yeah.

@chrisvittal chrisvittal force-pushed the indexedrvdspec-val-partitioner branch from 7a44673 to bfd521a Compare July 12, 2019 20:51
@chrisvittal
Copy link
Collaborator Author

This benchmark is in some ways bad. The real problem is in the compiler/orchestration, not in any execution. I feel like we need a _do_nothing() that doesn't execute any code, but runs a pipeline through everything in the compiler and stops short before anything that would submit a spark job.

@tpoterba
Copy link
Contributor

yeah, I agree. But we can also get that effect by having tiny data but big pipelines

@danking danking merged commit 84ec5c1 into hail-is:master Jul 12, 2019
Xophmeister added a commit to wtsi-hgi/hgi-cloud that referenced this pull request Aug 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants