search

AzureAD / azure-activedirectory-library-for-js

The code for ADAL.js and ADAL Angular has been moved to the MSAL.js repo. Please open any issues or PRs at the link below.
https://github.com/AzureAD/microsoft-authentication-library-for-js/tree/dev/maintenance/adal-angular
Apache License 2.0
627 stars 374 forks source link

The value of _supportsLocalStorage is not cached and causes a performance hit. #779

Closed maciek-codes closed 5 years ago

maciek-codes commented 6 years ago

While analyzing performance traces from Edge browser, I noticed that AuthenticationContext.prototype._supportsLocalStorage causes some CPU spikes and an extra perf hit related to the use of local storage. It appers that the method checks if window.localStorage is present and does tests it by writing, reading and deleteing a value under 'testkey' key.

While I understand this may be needed for some compatibility reasons (which are not known to me), I would strongly encourage caching the value of that check. I saw UI delays in Edge coming from login.microsoft.com due to higher pressure on local storage. Some other internal websites that use adal.js suffer more from CPU spikes cause by this redundant local storage access.

For every AuthenticationContext.prototype._getItem the penalty is an extra write, extra read and an extra delete operations. Same goes for every call to AuthenticationContext.prototype._saveItem.

maciek-codes commented 6 years ago

Created a pull request #786

nehaagrawal commented 5 years ago

@macqm Thank you for reporting this issue and submitting the PR. We are reviewing it and will update you here.

nehaagrawal commented 5 years ago

@macqm We have merged your PR. I will update this issue when we release a new version. Thanks for your contribution! Closing this issue for now.