8490: Include numeric types without Persister in "Add Filter from Attribute" menu by youngledo · Pull Request #699 · openjdk/jmc

youngledo · 2026-01-09T07:04:09Z

Problem

Attributes with numeric ContentTypes that don't have a Persister (such as primitive types: byte, short, int, long, float, double, char) are
excluded from the "Add Filter from Attribute" context menu in the Event Browser's filter editor.

Steps to reproduce:

Open a JFR file containing events with primitive numeric fields (e.g., Spring Framework's FlightRecorderStartupEvent with long eventId and
long parentId)
In Event Browser, right-click on a column → Select "Show Filter"
Right-click on the filter area → Select "Add Filter from Attribute"
Expected: All filterable attributes should appear in the menu
Actual: Primitive numeric type attributes are missing from the menu

Root Cause

The getPersistableAttributes() method in DataPageToolkit.java (line 1006-1007) filters attributes based on whether their ContentType has a
Persister:

.filter(a -> a.equals(JfrAttributes.EVENT_TYPE) || (a.getContentType() instanceof RangeContentType)
    || (a.getContentType().getPersister() != null))

However, primitive numeric types use these ContentTypes which don't have a Persister:

UnitLookup.RAW_NUMBER - used by all primitive numeric types (byte, short, int, long, float, double, char)
UnitLookup.RAW_LONG - used by some custom Long attributes
UnitLookup.COUNT, UnitLookup.INDEX, UnitLookup.IDENTIFIER - legacy numeric types

These types return null from getPersister() (see ContentType.java:99-101), causing them to be filtered out.

Why this is incorrect:

Even though these types cannot be persisted to strings, they fully support filtering via ItemFilters.equals(). Excluding them from the filter
menu is inconsistent with their actual capabilities.

Solution

Add a helper method isFilterableNumericType() to identify numeric ContentTypes that support filtering even without a Persister, and include them
in the filter:

private static boolean isFilterableNumericType(IAttribute<?> attribute) {
      org.openjdk.jmc.common.unit.ContentType<?> ct = attribute.getContentType();
      return ct.equals(UnitLookup.RAW_NUMBER) || ct.equals(UnitLookup.RAW_LONG)|| ct.equals(UnitLookup.COUNT) || ct.equals(UnitLookup.INDEX)
              || ct.equals(UnitLookup.IDENTIFIER);
  }

Update the filter condition to include these types:

  .filter(a -> a.equals(JfrAttributes.EVENT_TYPE)
      || (a.getContentType() instanceof RangeContentType)
      || (a.getContentType().getPersister() != null)
      || isFilterableNumericType(a))

Testing

Tested with Spring Framework's FlightRecorderStartupEvent which has primitive long fields:

Before fix: eventId and parentId attributes were missing from "Add Filter from Attribute" menu
After fix: All primitive numeric attributes appear correctly in the menu and filters work as expected

Attachment: app.jfr.zip

Related issues: spring-projects/spring-framework#36077 (comment)

Progress

Commit message must refer to an issue
Change must be properly reviewed (1 review required, with at least 1 Committer)

Issue

JMC-8490: Include numeric types without Persister in "Add Filter from Attribute" menu (Bug - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jmc.git pull/699/head:pull/699
$ git checkout pull/699

Update a local copy of the PR:
$ git checkout pull/699
$ git pull https://git.openjdk.org/jmc.git pull/699/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 699

View PR using the GUI difftool:
$ git pr show -t 699

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jmc/pull/699.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2026-01-09T07:05:30Z

👋 Welcome back youngledo! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2026-01-09T07:06:34Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

youngledo · 2026-01-09T07:06:55Z

I don't have access to JBS yet. Could someone please create an issue for this PR?

@aymane-harmaz Hello, Can you help me?
May I ask, what are the ways I can apply for this account?

aymane-harmaz · 2026-01-12T08:34:29Z

@youngledo Thanks for opening this PR.

I have opened a JBS ticket for you here : https://bugs.openjdk.org/browse/JMC-8490

In order to obtain write access to JBS you need to have the Author role, You can find more information about the process and requirements here : https://openjdk.org/guide/#becoming-an-author

mlbridge · 2026-01-12T10:04:27Z

Webrevs

thegreystone · 2026-01-30T19:39:43Z

The current PR will allow these attributes to appear in the menu, but users will likely hit errors when trying to use or save filters on them. I'd suggest either:

(Preferred) Add proper LeafContentType persisters to the numeric types in UnitLookup
(Minimal) Add null-safety handling in the filter persistence code alongside this change

Happy to discuss further or help with implementation!

youngledo · 2026-01-31T10:00:50Z

Welcome to vote. If there are no votes, I will implement according to the first suggestion.

openjdk · 2026-01-31T14:26:57Z

@youngledo Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

thegreystone · 2026-02-03T18:07:13Z

Will the removal of the .classpath files cause any trouble running JMC from within the Eclipse development environment? I am not perfectly sure, but I have a faint memory of a few of them being slightly specialized.

youngledo · 2026-02-04T01:45:54Z

Will the removal of the .classpath files cause any trouble running JMC from within the Eclipse development environment? I am not perfectly sure, but I have a faint memory of a few of them being slightly specialized.

Thank you for the reminder, I have recovered.

bridgekeeper · 2026-03-04T14:19:47Z

@youngledo This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

Copilot

Pull request overview

This PR addresses JMC-8490 by making primitive numeric attributes available in the Event Browser filter editor’s “Add Filter from Attribute” menu, aligning the menu contents with what can actually be filtered.

Changes:

Update numeric ContentTypes (e.g., RAW_NUMBER, RAW_LONG, COUNT/INDEX/IDENTIFIER) to be persistable by providing IPersister implementations.
Expand DataPageToolkit.getPersistableAttributes() to also include selected numeric content types even when no persister is present.
Minor UI/support tweaks (cell editor exception handling, small stream refactor, and .gitignore update).

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
core/org.openjdk.jmc.common/src/main/java/org/openjdk/jmc/common/unit/UnitLookup.java	Adds persisters/parsing for numeric content types that previously had no persister.
application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/common/DataPageToolkit.java	Includes numeric content types in the attribute list for “Add Filter from Attribute”.
application/org.openjdk.jmc.ui.celleditors/src/main/java/org/openjdk/jmc/ui/celleditors/CommonCellEditors.java	Handles additional parse failures from persisters.
application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/pages/EventBrowserPage.java	Minor refactor to method reference.
.gitignore	Ignores `.claude/`.

Comments suppressed due to low confidence (1)

application/org.openjdk.jmc.ui.celleditors/src/main/java/org/openjdk/jmc/ui/celleditors/CommonCellEditors.java:117

The two catch blocks here are identical. This can be simplified to a single multi-catch (QuantityConversionException | IllegalArgumentException) to avoid duplication and keep future changes consistent.

			} catch (QuantityConversionException ex) {
				errorDecorator.setDescriptionText(ex.getLocalizedMessage());
				errorDecorator.show();
				setValueValid(false);
			} catch (IllegalArgumentException ex) {
				errorDecorator.setDescriptionText(ex.getLocalizedMessage());
				errorDecorator.show();
				setValueValid(false);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…ibute" menu.

…ibute" menu(Update copyright year to 2026).

…ibute" menu(Apply Spotless formatting).

This commit adds IPersister implementation to the following numeric types: - RAW_NUMBER - RAW_LONG - COUNT - INDEX - IDENTIFIER These types previously had no Persister, which would cause errors when users tried to edit or save filters on them. By implementing the IPersister interface via LeafContentType, these types can now be properly persisted and restored. Changes: - Modified createRawNumber() to return a LeafContentType with full persister support - Modified createRawLong() to return a LeafContentType with full persister support - Modified createCount() to return a LeafContentType with full persister support - Modified createIdentifier() to return a LeafContentType with full persister support - Modified createIndex() to return a LeafContentType with full persister support - Added parseNumber() helper method to parse string representations of numbers The persister implementation supports: - persistableString(): converts Number values to strings for storage - parsePersisted(): parses strings back to Number objects - interactiveFormat(): user-friendly string representation - parseInteractive(): parses user input strings This fix resolves the issue where attributes with these numeric content types would appear in the "Add Filter from Attribute" menu but fail when users attempted to use or save filters on them.

This commit adds validation for empty and blank strings in the parseNumber() method and RAW_LONG persister methods to prevent NumberFormatException when users clear input fields in the filter editor. Changes: - parseNumber(): Check for null/empty string before parsing - createRawLong() parsePersisted(): Check for empty string before parsing - createRawLong() parseInteractive(): Check for empty string before parsing - Updated javadoc to document the NumberFormatException for empty strings This fixes the issue where editing a numeric filter value and clearing the input field would cause an unhandled NumberFormatException with the message "empty String" instead of a more user-friendly error.

Changed parseNumber() to always return Long (for integers) or Double (for decimals) instead of Integer/Long/Double hierarchy to ensure type compatibility when comparing filter values. Problem: - Previously parseNumber("100") returned Integer(100) - If actual data was Long(100L), comparison would fail because Integer(100).equals(Long(100L)) returns false - This caused filters to not match any data even when values existed Solution: - parseNumber() now returns Long for all integer values - Returns Double only for decimal values - This ensures Long.equals(Long) comparisons work correctly - All RAW_NUMBER, COUNT, INDEX, IDENTIFIER types now use consistent types

Modified both UnitLookup and CommonCellEditors to properly handle empty or invalid input when editing numeric type filters, preventing "Unhandled event loop exception" errors. Changes: 1. UnitLookup.java: - Added try-catch blocks to parsePersisted() and parseInteractive() methods - Catch NumberFormatException and throw IllegalArgumentException with clear message - Applied to RAW_NUMBER, RAW_LONG, COUNT, INDEX, and IDENTIFIER types 2. CommonCellEditors.java: - Added catch block for IllegalArgumentException and NumberFormatException - Displays error message in UI when user enters invalid value - Prevents unhandled exception from crashing the UI This ensures a user-friendly experience when editing numeric filters, especially when clearing input fields.

This file should not be tracked as it contains local settings. Added .claude/ to .gitignore to prevent future commits. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

This commit restores the .classpath files that were previously removed. These files are essential for JMC development in Eclipse IDE due to the project's hybrid nature as both a Maven and PDE/OSGi application.

This commit addresses code review feedback from Copilot and improves exception handling consistency across the codebase. Changes: - Simplify CommonCellEditors.java by merging duplicate catch blocks into a single multi-catch for QuantityConversionException and IllegalArgumentException - Update Long type persisters in UnitLookup.java to throw QuantityConversionException instead of IllegalArgumentException for consistency with other comparable types - Add IllegalArgumentException handling in PersistableItemFilter.readValue() to prevent uncaught runtime exceptions during state restore for malformed persisted filter values - Add PersisterRoundTripTest.java to verify correct persistence and restoration of primitive types (RAW_NUMBER, RAW_LONG, COUNT, INDEX, IDENTIFIER) and prevent regressions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

openjdk · 2026-03-05T10:31:33Z

@youngledo Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

Update copyright year in files modified by recent changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

bridgekeeper · 2026-04-02T17:16:18Z

@youngledo This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

youngledo · 2026-04-03T01:26:00Z

/keepalive

openjdk · 2026-04-03T01:26:30Z

@youngledo The pull request is being re-evaluated and the inactivity timeout has been reset.

openjdk · 2026-04-03T01:26:47Z

@youngledo this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout event-browser-menu
git fetch https://git.openjdk.org/jmc.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

openjdk · 2026-04-03T02:29:12Z

@youngledo Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

youngledo changed the title ~~Fix: Include numeric types without Persister in "Add Filter from Attribute" menu.~~ 8490: Include numeric types without Persister in "Add Filter from Attribute" menu. Jan 12, 2026

youngledo changed the title ~~8490: Include numeric types without Persister in "Add Filter from Attribute" menu.~~ 8490: Include numeric types without Persister in "Add Filter from Attribute" menu Jan 12, 2026

openjdk bot added the rfr label Jan 12, 2026

youngledo force-pushed the event-browser-menu branch from ddba415 to 1c1601c Compare January 31, 2026 14:26

thegreystone requested a review from Copilot March 4, 2026 18:20

Copilot started reviewing on behalf of thegreystone March 4, 2026 18:21 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

youngledo and others added 12 commits March 5, 2026 18:29

Fix: Include numeric types without Persister in "Add Filter from Attr…

9f4f776

…ibute" menu.

Fix: Include numeric types without Persister in "Add Filter from Attr…

2b5c11a

…ibute" menu(Update copyright year to 2026).

Fix: Include numeric types without Persister in "Add Filter from Attr…

afacf04

…ibute" menu(Apply Spotless formatting).

Remove .classpath files from version control

05d197e

Remove .claude/settings.local.json from version control

15c1ea9

This file should not be tracked as it contains local settings. Added .claude/ to .gitignore to prevent future commits. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Restore .classpath files required for Eclipse IDE development

7b0300b

This commit restores the .classpath files that were previously removed. These files are essential for JMC development in Eclipse IDE due to the project's hybrid nature as both a Maven and PDE/OSGi application.

youngledo force-pushed the event-browser-menu branch from f842f18 to 97dd422 Compare March 5, 2026 10:30

Update copyright year to 2026

e296f7d

Update copyright year in files modified by recent changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

openjdk bot added merge-conflict Pull request has merge conflict with target branch and removed merge-conflict Pull request has merge conflict with target branch labels Apr 3, 2026

Merge master

1166612

youngledo force-pushed the event-browser-menu branch from d42ad5e to 1166612 Compare April 3, 2026 02:28

Conversation

youngledo commented Jan 9, 2026 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Steps to reproduce:

Root Cause

Why this is incorrect:

Solution

Testing

Progress

Issue

Reviewing

Uh oh!

bridgekeeper bot commented Jan 9, 2026

Uh oh!

openjdk bot commented Jan 9, 2026

Uh oh!

youngledo commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aymane-harmaz commented Jan 12, 2026

Uh oh!

mlbridge bot commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

thegreystone commented Jan 30, 2026

Uh oh!

youngledo commented Jan 31, 2026

Uh oh!

openjdk bot commented Jan 31, 2026

Uh oh!

thegreystone commented Feb 3, 2026

Uh oh!

youngledo commented Feb 4, 2026

Uh oh!

bridgekeeper bot commented Mar 4, 2026

Uh oh!

Copilot AI left a comment • edited by bridgekeeper bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

openjdk bot commented Mar 5, 2026

Uh oh!

bridgekeeper bot commented Apr 2, 2026

Uh oh!

youngledo commented Apr 3, 2026

Uh oh!

openjdk bot commented Apr 3, 2026

Uh oh!

openjdk bot commented Apr 3, 2026

Uh oh!

openjdk bot commented Apr 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

youngledo commented Jan 9, 2026 •

edited by openjdk bot

Loading

youngledo commented Jan 9, 2026 •

edited

Loading

mlbridge bot commented Jan 12, 2026 •

edited

Loading

Copilot AI left a comment •

edited by bridgekeeper bot

Loading