Skip running color match regex, if not required

[pramilk]

Color Parsing is in a critial hot path and is executed multiple times while style parsing. This PR brings in a small optimization.

Before running a expensive color match regex run a cheap validation. Tests shows a 98% improvement if color is specified in HSL format. There will be some improvement when color is specified in rgb format as well.
Test: https://jsbench.me/4blhzc953l/1

Launch Checklist

• Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
• Briefly describe the changes in this PR.
• Link to related issues.
• Include before/after visuals or gifs if this PR includes visual changes.
• Write tests for all new functionality.
• Document any changes to public APIs.
• Post benchmark scores.
• Add an entry to CHANGELOG.md under the ## main section.

[wipfli]

Shouldn't CI run automatically for people who are not first-time contributors?

[HarelM]

Probably a first time in this repo.
But I think the setting are too strict for my taste (both in this repo, and in gl-js).
I would prefer that if the github user is not a new user the CI will run, and not only if the user had contributed in the past...

[HarelM]

I think package-lock.json needs to be updated as well.

[pramilk]

I think package-lock.json needs to be updated as well.

Sorry forgot about it. Updated.

[pramilk]

@HarelM, Can you please take a look.

[kajkal]

I stumbled across this change and it intrigued me.
I decided to make my own benchmark because I think the one attached to this PR is misleading by showing only hsl().

https://jsbench.me/mdli7ehoen/1

From my tests, it appears that, assuming hsl() is not the only format, and # and rgb are also used in similar proportions, this change degrades color parsing performance (I checked in chrome and firefox).

It seems that better performance boost would be if you simply put the regexp declarations in a higher scope. But in any case, I don't think the current color parsing performance is a big problem that critically needs to be optimized.

[HarelM]

Ahh interesting! :-)
Would be interesting to see what happens when most of the input is rgb or hex as I think they are more widely used, maybe along color names, IDK...
@pramilk any comments on this new findings?

[pramilk]

I had tested moving regex to higher scope and it did no showed much difference before making this change.

Here is updated test with Regex moved to setup

Testing for Hex and RGB
https://jsbench.me/4blhzc953l/2

Testing for just Hex
https://jsbench.me/4blhzc953l/3

Intent of the original test(https://jsbench.me/4blhzc953l/1) was to see if adding a cheap startswith validation will be much faster then a full regex match without any additional code. In Bing maps road style we have 158 #hex and 201 HSL. Maplibre default style has 94 #hex, 79 RGB and 11 HSL.

Testing with mixed color format, dilutes the test and does not test the change that was intended. Like having hex color will force test to also run its regex. Intend was to simply have a cheap way to test the color before running regex.

[kajkal]

I compared the entire parsing functions (had to minify code as it is at the jsbench limits):

https://jsbench.me/mdli7ehoen/2

The results made me think why my earlier test would suggest the opposite. And so I noticed that I added a for of loop for code that was intended to be in the body of a function and the early return spoiled the test completely 😩

So yes, I was wrong and this change indeed improves color parsing performance. Sorry for claiming otherwise 😔
And according to the test above regexps in a higher scope only slightly improves performance.

As a final note, I would like to add that I disagree with the statement that testing with mixed formats dilutes the test, in my opinion it is more valuable because it verifies the additional cost for the hex/rgb() paths, instead of focusing only on the benefits for the hsl() path.