Skip to content

Bound table row scans by getRowCount instead of exception termination where cheap #787

Description

@jeffjensen

Problem

Several table scans loop until RowOutOfBoundsException terminates them, even where the row count is cheaply available. The stack-trace cost of that pattern is addressed separately (#770); the exception-per-scan itself remains.

Status: not implemented. Investigated as part of #770; skipped entirely, for reasons stronger than the item's own gate.

What was investigated

This item's own gate required "profiling still shows residual cost after #770 fix" before implementing. This codebase has no profiling or benchmark infrastructure (no JMH, no timing tests — see performance-improvements.adoc Appendix A), so that condition has never been, and can't currently be, verified either way.

More importantly, the item's actual target callers were checked directly: DataSetProducerAdapter.produce(), RefreshOperation.execute(), and AbstractBatchOperation's empty-table check (isEmptyTable()) and its batch-insert loop. All three scan a generic ITable with for (int i = 0; ; i++) { ... }, relying on RowOutOfBoundsException to signal the end. All three can be handed a ForwardOnlyResultSetTable — a real, supported, non-default table implementation (ForwardOnlyResultSetTableFactory) used for large or streaming result sets — and that class's getRowCount() unconditionally throws UnsupportedOperationException.

Bounding these scans by getRowCount() unconditionally, as originally proposed, would break every one of these three call sites whenever a forward-only table is in use. That's a correctness blocker discovered during investigation, not merely an unproven-benefit gate, so no production code was changed.

Remaining fix need

No evidence this costs anything worth fixing today — the gate's own precondition (profiling showing residual cost after #770) has never been met and can't be without adding benchmark infrastructure this codebase doesn't have.

If it's ever revisited, an unconditional switch to getRowCount() bounds is unsafe (see above). A safe version would need to probe first: attempt table.getRowCount() once per scan (not per row) inside a try/catch for UnsupportedOperationException, use the count as the loop bound on success, and fall back to the original exception-terminated scan on failure. That would preserve forward-only/streaming table support while still saving the exception-per-scan cost for the common (non-forward-only) case. Not attempted as part of the current performance-improvements.adoc plan.

Proposed fix (original, superseded by the above)

Optional follow-up: where a count is cheap, loop to getRowCount() bounds instead of scanning until the exception. One commit per affected class; implement only if profiling still shows residual cost after #770 fix.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: datasetIDataSet, ITable, Column, dataset implementationstype: performancePerformance improvement

    Projects

    Status
    Backlog

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions