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

Proposed replacement for progress PR #1302

Merged
merged 11 commits into from
Mar 22, 2019
Merged
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 49 additions & 20 deletions src/main/java/htsjdk/samtools/util/AbstractProgressLogger.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,14 @@ abstract public class AbstractProgressLogger implements ProgressLoggerInterface
private long startTime;
private final NumberFormat fmt = new DecimalFormat("#,###");
private final NumberFormat timeFmt = new DecimalFormat("00");
private long processed;
private long lastStartTime;
private String lastChrom;
private int lastPos;
private long processed = 0;
// Set to -1 until the first record is added
private long lastStartTime = -1;
private String lastChrom = null;
private int lastPos = 0;
private String lastReadName = null;
private long countNonIncreasing = 0;
final private long PRINT_READ_NAME_THRESHOLD = 1000;
Copy link
Member

Choose a reason for hiding this comment

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

might as well make this static

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure


/**
* Construct an AbstractProgressLogger.
Expand Down Expand Up @@ -60,18 +64,26 @@ private synchronized void record() {
if (this.lastChrom == null) readInfo = "*/*";
else readInfo = this.lastChrom + ":" + fmt.format(this.lastPos);

final String rnInfo;

if (lastReadName != null && countNonIncreasing > PRINT_READ_NAME_THRESHOLD) {
rnInfo = ". Last read name: " + lastReadName;
} else {
rnInfo = "";
}

final long n = (this.processed % this.n == 0) ? this.n : this.processed % this.n;

log(this.verb, " ", processed, " " + noun + ". Elapsed time: ", elapsed, "s. Time for last ", fmt.format(n),
": ", period, "s. Last read position: ", readInfo);
": ", period, "s. Last read position: ", readInfo, rnInfo);
}

/**
* Logs the last last record if it wasn't previously logged.
* @return boolean true if logging was triggered, false otherwise
*/
public synchronized boolean log() {
if (this.processed % this.n != 0) {
if (processed % this.n != 0) {
record();
return true;
}
Expand All @@ -80,14 +92,22 @@ public synchronized boolean log() {
}
}

@Override
public synchronized boolean record(final String chrom, final int pos) {
this.lastChrom = chrom;
this.lastPos = pos;
if (this.lastStartTime == -1) {
this.lastStartTime = System.currentTimeMillis();
protected synchronized boolean record(final String chrom, final int pos, final String rname) {
lastChrom = chrom;
if (pos >= lastPos) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

only increase counter when chrom name is unchanged, and do not decrease counter.

countNonIncreasing = Math.max(0, countNonIncreasing--);
} else {
countNonIncreasing++;
if (countNonIncreasing == PRINT_READ_NAME_THRESHOLD) {
log("Seen many non-increasing record positions. Printing Readnames as well.");
}
}
lastPos = pos;
lastReadName = rname;
if (lastStartTime == -1) {
lastStartTime = System.currentTimeMillis();
}
if (++this.processed % this.n == 0) {
if (++processed % n == 0) {
record();
return true;
}
Expand All @@ -96,25 +116,31 @@ public synchronized boolean record(final String chrom, final int pos) {
}
}

@Override
public synchronized boolean record(final String chrom, final int pos) {
return record(chrom, pos, null);
}

/**
* Records that a given record has been processed and triggers logging if necessary.
* @return boolean true if logging was triggered, false otherwise
*/
@Override
public synchronized boolean record(final SAMRecord rec) {
if (SAMRecord.NO_ALIGNMENT_REFERENCE_NAME.equals(rec.getReferenceName())) {
return record(null, 0);
}
else {
return record(rec.getReferenceName(), rec.getAlignmentStart());
return record(null, 0, rec.getReadName());
} else {
return record(rec.getReferenceName(), rec.getAlignmentStart(), rec.getReadName());
}
}

/** Records multiple SAMRecords and triggers logging if necessary. */
@Override
public boolean record(final SAMRecord... recs) {
boolean triggered = false;
for (final SAMRecord rec : recs) triggered = record(rec) || triggered;
for (final SAMRecord rec : recs) {
triggered = record(rec) || triggered;
}
return triggered;
}

Expand All @@ -132,13 +158,16 @@ public void reset() {
lastStartTime = -1;
lastChrom = null;
lastPos = 0;
lastReadName = null;
}

/** Left pads a string until it is at least the given length. */
private String pad (String in, final int length) {
while (in.length() < length) {
in = " " + in;
final StringBuilder inBuilder = new StringBuilder(in);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

preallocate the builder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rewrite to avoid silliness.

while (inBuilder.length() < length) {
inBuilder.insert(0, " ");
}
in = inBuilder.toString();

return in;
}
Expand Down