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

Expensive init due to inefficient UserAgent parsing #210

Open
AaronO opened this issue Nov 26, 2019 · 0 comments
Open

Expensive init due to inefficient UserAgent parsing #210

AaronO opened this issue Nov 26, 2019 · 0 comments
Labels
enhancement New feature or request

Comments

@AaronO
Copy link

AaronO commented Nov 26, 2019

Cross posting this from faisalman/ua-parser-js#401 (I know this SDK uses your own fork, but issues are disabled on your fork):

A big chunk of Amplitude's initialization time is spent in ua-parser's getResult(), see profile below:

image

This profile is for Chrome running on macOS on a MacBook Pro.

Given that our detection logic is basically, scan through the userAgent string with a list of regexes until one matches then stop. The worst case is O(n) where n is the number of regexes (per feature we're detecting, browser/os/device/etc ...)

We can't optimize the worst case scenario without radically changing this library's design (e.g: to a one-pass parse of the userAgent), but we can significantly improve the performance of the common case for popular configurations.

If we focus on the list of os regexes, macOS & iOS are near the end, whereas less popular OSes (e.g: blackberry, nintendo/playstation, ...), which means the rgx() call with have to test all those lesser popular platforms before getting to iOS/macOS.

The same is true for the browser regexes, Chrome is near the end of that list despite being the most popular browser on the market.

Additionally there doesn't seem to be an overlap between regexes that would require running some of them before others for correctness. So it seems like a no-brainer to sort regexes by their popularity (Chrome, Edge, Firefox, Safari, ...), (Windows, Android, iOS, Mac, ...)

Further improvements

Additionally it seems like this SDK only uses the outputs of getBrowser() and getOS(), so we could remove the getDevice() call/code, to make it even faster.

@haoliu-amp haoliu-amp added the enhancement New feature or request label Mar 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants