Skip to content

Commit

Permalink
Remove unnecessary expanded region query and small optimizations+perf…
Browse files Browse the repository at this point in the history
…ormance improvements for alignments tracks (#3279)
  • Loading branch information
cmdcolin committed Oct 21, 2022
1 parent e99004d commit 87dc7ab
Show file tree
Hide file tree
Showing 14 changed files with 182 additions and 210 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import deepEqual from 'fast-deep-equal'

// layouts
import GranularRectLayout from '../../util/layouts/GranularRectLayout'
import MultiLayout from '../../util/layouts/MultiLayout'
import { SerializedLayout, BaseLayout } from '../../util/layouts/BaseLayout'
import PrecomputedLayout from '../../util/layouts/PrecomputedLayout'

// other
import FeatureRendererType, {
RenderArgs as FeatureRenderArgs,
RenderArgsSerialized as FeatureRenderArgsSerialized,
Expand All @@ -11,7 +16,6 @@ import FeatureRendererType, {
ResultsDeserialized as FeatureResultsDeserialized,
} from './FeatureRendererType'
import { getLayoutId, Region, Feature } from '../../util'
import { SerializedLayout, BaseLayout } from '../../util/layouts/BaseLayout'
import { readConfObject, AnyConfigurationModel } from '../../configuration'
import SerializableFilterChain from './util/serializableFilterChain'
import RpcManager from '../../rpc/RpcManager'
Expand Down
109 changes: 44 additions & 65 deletions packages/core/util/layouts/GranularRectLayout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,30 +34,19 @@ interface RowState<T> {
}
// a single row in the layout
class LayoutRow<T> {
private padding: number
private padding = 1

private allFilled?: Record<string, T> | string

private widthLimit: number
private widthLimit = 1_000_000

private rowState?: RowState<T>

constructor() {
this.padding = 1
this.widthLimit = 1000000

// this.rowState.offset is the offset of the bits array relative to the genomic coordinates
// (modified by pitchX, but we don't know that in this class)
// this.rowState.bits is the array of items in the layout row, indexed by (x - this.offset)
// this.rowState.min is the leftmost edge of all the rectangles we have in the layout
// this.rowState.max is the rightmost edge of all the rectangles we have in the layout
}

// log(msg: string): void {
// // if (this.rowNumber === 0)
// // eslint-disable-next-line no-console
// console.log(`r${this.rowNumber}: ${msg}`)
// }
// this.rowState.bits is the array of items in the layout row, indexed by (x - this.offset)
// this.rowState.min is the leftmost edge of all the rectangles we have in the layout
// this.rowState.max is the rightmost edge of all the rectangles we have in the layout
// this.rowState.offset is the offset of the bits array relative to the genomic coordinates
// (modified by pitchX, but we don't know that in this class)

setAllFilled(data: Record<string, T> | string): void {
this.allFilled = data
Expand All @@ -67,64 +56,49 @@ class LayoutRow<T> {
if (this.allFilled) {
return this.allFilled
}
if (!this.rowState) {
return undefined
}

if (this.rowState.min === undefined) {
return undefined
}
if (x < this.rowState.min) {
return undefined
}
if (x >= this.rowState.max) {
if (
this.rowState?.min === undefined ||
x < this.rowState.min ||
x >= this.rowState.max
) {
return undefined
}
const offset = x - this.rowState.offset
// if (offset < 0)
// debugger
// if (offset >= this.rowState.bits.length)
// debugger
return this.rowState.bits[offset]
return this.rowState.bits[x - this.rowState.offset]
}

isRangeClear(left: number, right: number): boolean {
isRangeClear(left: number, right: number) {
if (this.allFilled) {
return false
}

if (!this.rowState) {
return true
}

const { min, max } = this.rowState

if (right <= min || left >= max) {
if (
this.rowState === undefined ||
right <= this.rowState.min ||
left >= this.rowState.max
) {
return true
}
const { min, max, offset, bits } = this.rowState

// TODO: check right and middle before looping
const maxX = Math.min(max, right)
let x = Math.max(min, left)
for (; x < right && x < maxX; x += 1) {
if (this.getItemAt(x)) {
return false
}
const maxX = Math.min(max, right) - offset
let flag = true
for (let x = Math.max(min, left) - offset; x < maxX && flag; x++) {
flag = bits[x] === undefined
}

return true
return flag
}

// NOTE: this.rowState.min, this.rowState.max, and this.rowState.offset are
// interbase coordinates
initialize(left: number, right: number): RowState<T> {
// NOTE: this.rowState.min, this.rowState.max, and this.rowState.offset are interbase coordinates
const rectWidth = right - left
return {
offset: left - rectWidth,
min: left,
max: right,
bits: new Array(3 * rectWidth),
}
// this.log(`initialize ${this.rowState.min} - ${this.rowState.max} (${this.rowState.bits.length})`)
}

addRect(rect: Rectangle<T>, data: Record<string, T> | string): void {
Expand Down Expand Up @@ -415,13 +389,13 @@ export default class GranularRectLayout<T> implements BaseLayout<T> {
return top * this.pitchY
}

collides(rect: Rectangle<T>, top: number): boolean {
collides(rect: Rectangle<T>, top: number) {
const { bitmap } = this

const maxY = top + rect.h
for (let y = top; y < maxY; y += 1) {
const row = bitmap[y]
if (row && !row.isRangeClear(rect.l, rect.r)) {
if (row !== undefined && !row.isRangeClear(rect.l, rect.r)) {
return true
}
}
Expand All @@ -432,7 +406,7 @@ export default class GranularRectLayout<T> implements BaseLayout<T> {
/**
* make a subarray if it does not exist
*/
private autovivifyRow(bitmap: LayoutRow<T>[], y: number): LayoutRow<T> {
private autovivifyRow(bitmap: LayoutRow<T>[], y: number) {
let row = bitmap[y]
if (!row) {
if (y > this.hardRowLimit) {
Expand All @@ -448,7 +422,7 @@ export default class GranularRectLayout<T> implements BaseLayout<T> {
return row
}

addRectToBitmap(rect: Rectangle<T>): void {
addRectToBitmap(rect: Rectangle<T>) {
if (rect.top === null) {
return
}
Expand All @@ -475,7 +449,7 @@ export default class GranularRectLayout<T> implements BaseLayout<T> {
* Given a range of X coordinates, deletes all data dealing with
* the features.
*/
discardRange(left: number, right: number): void {
discardRange(left: number, right: number) {
// console.log( 'discard', left, right );
const pLeft = Math.floor(left / this.pitchX)
const pRight = Math.floor(right / this.pitchX)
Expand All @@ -488,11 +462,11 @@ export default class GranularRectLayout<T> implements BaseLayout<T> {
}
}

hasSeen(id: string): boolean {
hasSeen(id: string) {
return this.rectangles.has(id)
}

getByCoord(x: number, y: number): Record<string, T> | string | undefined {
getByCoord(x: number, y: number) {
const pY = Math.floor(y / this.pitchY)
const row = this.bitmap[pY]
if (!row) {
Expand All @@ -502,23 +476,28 @@ export default class GranularRectLayout<T> implements BaseLayout<T> {
return row.getItemAt(pX)
}

getByID(id: string): RectTuple | undefined {
getByID(id: string) {
const r = this.rectangles.get(id)
if (r) {
const t = (r.top as number) * this.pitchY
return [r.l * this.pitchX, t, r.r * this.pitchX, t + r.originalHeight]
return [
r.l * this.pitchX,
t,
r.r * this.pitchX,
t + r.originalHeight,
] as RectTuple
}

return undefined
}

getDataByID(id: string): unknown {
getDataByID(id: string) {
return this.rectangles.get(id)?.data
}

cleanup(): void {}
cleanup() {}

getTotalHeight(): number {
getTotalHeight() {
return this.pTotalHeight * this.pitchY
}

Expand Down
12 changes: 3 additions & 9 deletions packages/core/util/layouts/MultiLayout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,15 @@ import { BaseLayout, SerializedLayout } from './BaseLayout'
export default class MultiLayout<SUB_LAYOUT_CLASS extends BaseLayout<T>, T> {
subLayouts: Map<string, SUB_LAYOUT_CLASS> = new Map()

subLayoutConstructorArgs: Record<string, any> = {}

/**
* layout class that just keeps a number of named sub-layouts.
* basically just a fancier
* `{ layout1: new GranularRectLayout(), layout2: new GranularRectLayout() ...}`
*/
constructor(
public SubLayoutClass: new (...args: any[]) => SUB_LAYOUT_CLASS,
layoutArgs: Record<string, any> = {},
) {
this.subLayouts = new Map()
this.subLayoutConstructorArgs = layoutArgs
}
public subLayoutConstructorArgs: Record<string, any> = {},
) {}

getDataByID(id: string): unknown {
for (const layout of this.subLayouts.values()) {
Expand Down Expand Up @@ -53,8 +48,7 @@ export default class MultiLayout<SUB_LAYOUT_CLASS extends BaseLayout<T>, T> {
}

discardRange(layoutName: string, left: number, right: number) {
const layout = this.subLayouts.get(layoutName)
return layout && layout.discardRange(left, right)
return this.subLayouts.get(layoutName)?.discardRange(left, right)
}

toJSON() {
Expand Down
78 changes: 39 additions & 39 deletions plugins/alignments/src/BamAdapter/BamAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { openLocation } from '@jbrowse/core/util/io'
import { ObservableCreate } from '@jbrowse/core/util/rxjs'
import { Feature } from '@jbrowse/core/util/simpleFeature'
import { toArray } from 'rxjs/operators'
import { readConfObject } from '@jbrowse/core/configuration'
import BamSlightlyLazyFeature from './BamSlightlyLazyFeature'

interface Header {
Expand All @@ -21,51 +20,52 @@ export default class BamAdapter extends BaseFeatureDataAdapter {
private samHeader?: Header

private setupP?: Promise<Header>

protected configured?: Promise<{
private configureP?: Promise<{
bam: BamFile
sequenceAdapter?: BaseFeatureDataAdapter
}>

// derived classes may not use the same configuration so a custom
// configure method allows derived classes to override this behavior
protected async configure() {
if (!this.configured) {
const bamLocation = readConfObject(this.config, 'bamLocation')
const location = readConfObject(this.config, ['index', 'location'])
const indexType = readConfObject(this.config, ['index', 'indexType'])
const bam = new BamFile({
bamFilehandle: openLocation(bamLocation, this.pluginManager),
csiFilehandle:
indexType === 'CSI'
? openLocation(location, this.pluginManager)
: undefined,
baiFilehandle:
indexType !== 'CSI'
? openLocation(location, this.pluginManager)
: undefined,

// chunkSizeLimit and fetchSizeLimit are more troublesome than
// helpful, and have given overly large values on the ultra long
// nanopore reads even with 500MB limits, so disabled with infinity
chunkSizeLimit: Infinity,
fetchSizeLimit: Infinity,
yieldThreadTime: Infinity,
})
protected async configurePre() {
const bamLocation = this.getConf('bamLocation')
const location = this.getConf(['index', 'location'])
const indexType = this.getConf(['index', 'indexType'])
const pm = this.pluginManager
const csi = indexType === 'CSI'
const bam = new BamFile({
bamFilehandle: openLocation(bamLocation, pm),
csiFilehandle: csi ? openLocation(location, pm) : undefined,
baiFilehandle: !csi ? openLocation(location, pm) : undefined,

// chunkSizeLimit and fetchSizeLimit are more troublesome than
// helpful, and have given overly large values on the ultra long
// nanopore reads even with 500MB limits, so disabled with infinity
chunkSizeLimit: Infinity,
fetchSizeLimit: Infinity,
yieldThreadTime: Infinity,
})

const adapterConfig = readConfObject(this.config, 'sequenceAdapter')
if (adapterConfig && this.getSubAdapter) {
this.configured = this.getSubAdapter(adapterConfig).then(
({ dataAdapter }) => ({
bam,
sequenceAdapter: dataAdapter as BaseFeatureDataAdapter,
}),
)
} else {
this.configured = Promise.resolve({ bam })
const adapterConfig = this.getConf('sequenceAdapter')
if (adapterConfig && this.getSubAdapter) {
const { dataAdapter } = await this.getSubAdapter(adapterConfig)
return {
bam,
sequenceAdapter: dataAdapter as BaseFeatureDataAdapter,
}
} else {
return { bam }
}
}

protected async configure() {
if (!this.configureP) {
this.configureP = this.configurePre().catch(e => {
this.configureP = undefined
throw e
})
}
return this.configured
return this.configureP
}

async getHeader(opts?: BaseOptions) {
Expand Down Expand Up @@ -231,7 +231,7 @@ export default class BamAdapter extends BaseFeatureDataAdapter {
// @ts-ignore
if (bam.index.filehandle !== '?') {
const bytes = await bytesForRegions(regions, bam)
const fetchSizeLimit = readConfObject(this.config, 'fetchSizeLimit')
const fetchSizeLimit = this.getConf('fetchSizeLimit')
return { bytes, fetchSizeLimit }
} else {
return super.estimateRegionsStats(regions, opts)
Expand All @@ -241,7 +241,7 @@ export default class BamAdapter extends BaseFeatureDataAdapter {
freeResources(/* { region } */): void {}

// depends on setup being called before the BAM constructor
refIdToName(refId: number): string | undefined {
refIdToName(refId: number) {
return this.samHeader?.idToName[refId]
}
}
Loading

0 comments on commit 87dc7ab

Please sign in to comment.