Skip to content

[GTK] Fetch List items once in setSelection(String[]) - #3657

Merged
akurtakov merged 1 commit into
eclipse-platform:masterfrom
vogella:gtk-list-indexof
Oct 5, 2026
Merged

akurtakov merged 1 commit into
eclipse-platform:masterfrom
vogella:gtk-list-indexof

Conversation

@vogella

@vogella vogella commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

List.setSelection(String[]) called indexOf for every string, and each indexOf copied all items from the native model, which made it O(m*n) native string fetches. It now fetches the items once; selecting 100 strings in a list of 50,000 items drops from about 3 s to 33 ms, and in a list of 10,000 items from about 590 ms to 7 ms. indexOf(String, int) also returns -1 for a negative start as its Javadoc specifies, instead of throwing ArrayIndexOutOfBoundsException.

@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (linux)

  109 files  ±0    109 suites  ±0   14m 48s ⏱️ -7s
4 637 tests ±0  4 402 ✅ ±0  235 💤 ±0  0 ❌ ±0 
3 482 runs  ±0  3 393 ✅ ±0   89 💤 ±0  0 ❌ ±0 

Results for commit fd12ef5. ± Comparison against base commit 63a0717.

♻️ This comment has been updated with latest results.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The corrected negative-start behavior needs regression coverage.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
What changed in this PR

Optimizes GTK List selection and corrects negative-start lookup behavior.

Changes:

  • Reuses one item snapshot during string-array selection.
  • Returns -1 for negative indexOf starts.
File Description
List.java Optimizes lookups and validates starting indices.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

setSelection(String[]) called indexOf per string, and each indexOf copied all items natively; it now fetches them once. indexOf also returns -1 for an out-of-range start as documented instead of throwing.

Assisted-by: multiple AI agents and layers of automated tooling 🤖
@vogella

vogella commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Fixed

@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Test Results

  212 files  ±0    212 suites  ±0   29m 3s ⏱️ + 1m 18s
4 974 tests ±0  4 946 ✅ ±0   28 💤 ±0  0 ❌ ±0 
7 221 runs  ±0  7 027 ✅ ±0  194 💤 ±0  0 ❌ ±0 

Results for commit 9e0a71b. ± Comparison against base commit 8b89d66.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The focused optimization preserves selection behavior and the index correction matches the documented contract.

Review effort: Balanced
Findings: None

Resolved since last review (1)

@akurtakov
akurtakov merged commit e2d1f88 into eclipse-platform:master Oct 5, 2026
22 checks passed
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.

3 participants