Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(56)

Issue 6368116: decodeURI and decodeURIComponent speedup (Closed)

Created:
9 years, 10 months ago by homm86
Modified:
9 years, 7 months ago
CC:
v8-dev
Visibility:
Public.

Description

Test code: var time = new Date(); for (var i = 0; i < 100000; i++) { decodeURIComponent(test_string); } print(new Date() - time); I have next results for new and old version of decodeURIComponent implementation: test_string = '%D0%BE%D0%BC%D0%BC?%D0%B1%D0%B0%D1%88=%D0%BE%D1%80%D0%B3&a=b&b=s&a=üń̈ï¨cödéwewe&url=http://example.com/images/avatar.jpg'; Old result: 714 ms New result: 524 ms test_string = '%D1%81%D1%83%D0%BF%D0%B5%D1%80%20%D1%8E%D1%80%D0%BB?%D0%BC%D0%B0%D0%BC%D0%B0%20%D0%BC%D1%8B%D0%BB%D0%B0%20%D1%80%D0%B0%D0%BC%D1%83&нёмног̈о юнико́да'; Old result: 1090 ms New result: 583 ms Speedup from 1.5 to 2 times depend of test string.

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -20 lines) Patch
M src/uri.js View 5 chunks +8 lines, -20 lines 2 comments Download

Messages

Total messages: 1 (0 generated)
Vyacheslav Egorov (Chromium)
9 years, 10 months ago (2011-02-07 10:32:35 UTC) #1
LGTM

Please add yourself to the AUTHORS file and fix one nit (see below, regarding
the names of variables).

After that I will land the patch.

Thanks for contributing!

http://codereview.chromium.org/6368116/diff/1/src/uri.js
File src/uri.js (right):

http://codereview.chromium.org/6368116/diff/1/src/uri.js#newcode95
src/uri.js:95: var m = HexValueOf(code2);
Please give more meaningful names to this variables.

For example: high and low.

http://codereview.chromium.org/6368116/diff/1/src/uri.js#newcode99
src/uri.js:99: return n * 16 + m;
Have you considered using

(high << 4) | low 

instead of

n * 16 + m?

It might be slightly faster.

Powered by Google App Engine
This is Rietveld 408576698