Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WHATWG URL API performance issue #3846

Open
SukkaW opened this issue Nov 8, 2019 · 0 comments

Comments

@SukkaW
Copy link
Member

@SukkaW SukkaW commented Nov 8, 2019

Since hexojs/hexo-util#114, we start to mitigate deprecated Legacy URL API to newer WHATWG URL API. Current status of mitigation can be found at this project board.

In order to fix #3796 (comment), I mitigate external_link filter from Legacy URL API to WHATWG URL API after #3812 & hexojs/hexo-util#119.

But immediately a critical performance issue came up after #3812 is merged (see #3833). @curbengh trying to revert #3812 in #3834) and it seems clear that WHATWG API is the cause.

I have carried out series of test trying to optimize external_link filter (#3839). During the research of WHATWG URL API, I have noticed a comment from maintainer of BootstrapCDN: MaxCDN/bootstrapcdn#1391

While deprecated, it's much cleaner to use this when we have relative URLs. It also appears to be a lot faster.

So I bring up a benchmark:

const Benchmark = require('benchmark');
const Suite = new Benchmark.Suite;
const { parse, URL } = require('url');
const urlObj = (str) => {
  try {
    return new URL(str);
  } catch (e) {
    return str;
  }
};
 
// add tests
Suite.add('Legacy URL parse()', () => {
  parse('https://skk.moe/path/to/something');
  parse('/path/to/something');
}).add('WHATWG URL API', () => {
  new URL('https://skk.moe/path/to/something');
  new URL('/path/to/something', 'https://skk.moe/');
}).add('safe WHATWG URL', () => {
  urlObj('https://skk.moe/path/to/something');
  urlObj('/path/to/something');
}).on('cycle', function(event) {
  console.info(String(event.target));
}).on('complete', function() {
  console.info('Fastest is ' + this.filter('fastest').map('name'));
}).run();

Result is clear, Legacy URL API is at least 4x faster than WHATWG URL API. And the approach (urlObj()) we came up to handle relative URL is even slower:

image

Based on the benchmark, I bring up 20d4ae4 in #3839 , avoid urlObj() by passing a base to new URL() to handle relative path. The generation performance backs to normal after this commit.

IMHO, although WHATWG URL API is powerful and safer, since the Legacy URL API is much faster and quite useful when handling relative path, we should use WHATWG URL API only when necessary.

Another idea is we should try to avoid urlObj() approach because try ... catch (err) ... is just too expensive. Takes hexojs/hexo-util#114 as an example, for those function that requires bind(hexo)(like full_url_for() and url_for()), we should pass hexo.config.url as a base to new URL() instead of using urlObj().

@SukkaW SukkaW added the discussion label Nov 8, 2019
@SukkaW SukkaW added this to To do in Drop legacy URL API via automation Nov 8, 2019
@SukkaW SukkaW changed the title WHATWG URL API performance WHATWG URL API performance issue Nov 8, 2019
@SukkaW SukkaW added the #perfmatters label Nov 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
2 participants
You can’t perform that action at this time.