[PATCH] minor regex cleanup: use switch for enum

[David Lloyd]

On Mon, Apr 23, 2018 at 7:42 PM, Isaac Levy <isaac.r.levy at gmail.com> wrote:
> On Mon, Apr 23, 2018 at 5:18 PM David Lloyd <david.lloyd at redhat.com> wrote:
>>
>> FWIW I strongly doubt this will improve performance; probably the
>> opposite in fact, as IIRC an enum switch generates an extra class
>> (though perhaps this has changed).  The original code was quite
>> compact and utilized identity comparisons, and given there are only
>> three alternatives it probably was able to exploit branch prediction
>> as well (if such a thing even matters in this context).
>
>
> Well, there are enum switches on the same enum elsewhere in the class, so
> should those instead be replaced by if checks?

I think that any change that's made for performance should be tested
against performance regression, generally speaking.  My personal
understanding is that, when there are a small number of alternatives,
an identity "if" tree can perform slightly better in some cases
because HotSpot C2 gathers and utilizes statistics about branches
taken in these cases; for switch, it (still IIRC) does not do so.
Given that this is regex, it might be worth testing.

> A larger change could remove this branch entirely, with different classes for each of the types, which
> are known during compile.

If that is beneficial.  However every new class adds metaspace usage
and (in this case) some kind of polymorphic dispatch, so you'd want to
be fairly confident that in a typical application, only one of the two
would be loaded, or that there would be some other significant gain,
as there is definitely a cost.  Everything has a cost, especially in
hot perf-sensitive code.

Anyway I'm not a reviewer but these are considerations that I would
have were I contributing the change.  JMH might be of use.

-- 
- DML