Thanks for the PR. This is adding 63 bytes to the gzipped minified build, we consider that a significant number. As I suggested in the other thread, can you try removing the branch that leverages nativetrim when available to decrease the size impact? Are there any issues with this strategy?

To verify the size increase, please runnpm run build locally on the3.x-stable branch and then on your PR rebased over that branch; you should see at the end what's the size of your current build compared to3.x-stable - the number 68 I posted above.

can you try removing the branch that leverages native trim when available to decrease the size impact?

Do you mean I should completely remove native trim approach?

Do you mean I should completely remove native trim approach?

Yes, that was the idea. Are there any issues with this approach? As I understand, your regex changes should resolve the performance issues that you reported.

You can also try the version without a nativetrim on a separate branch, just to measure what size impact would this version have while still being able to go back to the version you currently proposed if needed.

Native trim should be faster than any regexp, and I think the recent browsers should not require the regexp.
Frankly speaking, I think going for native trim is worth doing as it would make the webpages faster for the end-user.

Just in case:

#baseline   raw     gz Sizes289641  85451 dist/jquery.js 89653  30945 dist/jquery.min.js# with regexp only (without native trim)   raw     gz Sizes290048  85627 dist/jquery.js 89691  30959 dist/jquery.min.js  +407   +176 dist/jquery.js   +38    +14 dist/jquery.min.js# regexp and native trim   raw     gz Sizes290297  85690 dist/jquery.js 89812  31008 dist/jquery.min.js  +656   +239 dist/jquery.js  +159    +63 dist/jquery.min.js# regexp, single trim function that checks useDefaultTrim always   raw     gz Sizes290226  85687 dist/jquery.js 89781  31006 dist/jquery.min.js  +585   +236 dist/jquery.js  +128    +61 dist/jquery.min.js

Here's a sample benchmark:https://jsbench.me/oml5j9kjfu/1

Note that even the optimized regex still needs to walk over the entire string while the native implementation can walk the string backwards.

Results on my machine are:

trim 1000 char text like__________ __________ __________ __________ __________ __________ __________ __________ __________ __________ __________ __________ ...

letn=1;letwordLen=10;letnumWords=100;letspaceAround=1;letmid=('_'.repeat(wordLen)+' ').repeat(numWords)+'_';letsrc=' '.repeat(spaceAround)+mid+' '.repeat(spaceAround);letltrim=/^[\s\uFEFF\xA0]+/;letrtrim=/([^\s\uFEFF\xA0])[\s\uFEFF\xA0]+$/;

native: 16M ops/sec ~ 0.06 ms/trimregex: 700K ops/sec ~ 1.4 ms/trim (~23 times slower)

trim' '.repeat(1000) + '_' + ' '.repeat(1000)

native: 750K ops/sec ~ 1.3 ms/trimregex: 680K ops/sec ~ 1.47 ms/trim (13% slower)

trim' _ '

native: 85M ops/sec ~ 13 us/trimregex: 5.5M ops/sec ~ 180 us/trim (~13 times slower)

Frankly speaking, I think the results show that performance improvement is worth the added 63 bytes.
So I believe regex-based implementation could be a not-so-bad workaround for stale browsers, while native trim would save users's CPU and battery (which is more important than 63-byte transfer).

WDYT?

Here's profiling of JIRA issue I run into. I'm running Chrome 103.0.5060.114 on Apple M1 Max. There are twojquery.trim executions, and each of them takes 5 seconds.

I've merged functions into one:

   raw     gz Sizes290221  85685 dist/jquery.js 89781  31006 dist/jquery.min.js  +580   +234 dist/jquery.js  +128    +61 dist/jquery.min.js

I've createddeprecated/support, and the size is

   raw     gz Sizes290171  85668 dist/jquery.js 89767  30997 dist/jquery.min.js  +530   +217 dist/jquery.js  +114    +52 dist/jquery.min.js

I wonder: would updating jQuery with this patch help here, considering that JIRA would have to update the included jQuery?

Of course, it would definitely help.
If I remember well, then the optimized regex improves the performance from 5 seconds to 5 milliseconds. Unfortunately, the input data has sensitive information, and I would like to avoid spending time distilling the test case since the optimization is kind of obvious.

Adding 29 bytes for performance that only matters with pathological input seems like too much to me, but I would be comfortable with stillimproving performance by orders of magnitude for the earlier 6-byte increase:

@@ -15,7 +15,7 @@ define( [  // Support: Android <=4.0 only // Make sure we trim BOM and NBSP-var rtrim = /^[\s\uFEFF\xA0]+|[\s\uFEFF\xA0]+$/g;+var rtrim = /^[\s\uFEFF\xA0]+|([^\s\uFEFF\xA0])[\s\uFEFF\xA0]+$/g;  // Bind a function to a context, optionally partially applying any // arguments.@@ -82,6 +82,6 @@ jQuery.isNumeric = function( obj ) { jQuery.trim = function( text ) {        return text == null ?                "" :-               ( text + "" ).replace( rtrim, "" );+               ( text + "" ).replace( rtrim, "$1" ); }; } );

Thanks for your contribution@vlsi! We discussed this in the meeting and we'd prefer to go with@gibson042's shorter solution and not addsupport.trim to 3.x this late in the game. If you can make that change, we'd be happy to get this into 3.6.1.

For reference, I've captured the problematic text (~900KiB).

Old regexp takes ~5 seconds.
The updated one takes 3ms.

For trivial cases (e.g.trim('<div>')), the updated regexp executes with a rate of ~11M/sec (the native executes at a rate of 130M/sec).
In other words, 10K trims would consume 1ms of CPU time.

@gibson042 since your "Request changes" review is pending - can you have a look and approve if you have no remarks? This now implements your +6 bytes proposal, as I understand.

Landed, thanks! This change will be included in the upcoming3.6.1 release.