Skip to content
This repository has been archived by the owner on Jan 5, 2019. It is now read-only.

Util methods #38

Merged
merged 4 commits into from Feb 1, 2016
Merged

Util methods #38

merged 4 commits into from Feb 1, 2016

Conversation

djmitche
Copy link
Contributor

Got another one for you, @jonasfj

resolve(scopes) {
let granted = dfa.sortScopesForMerge(_.clone(scopes));
let granted = dfa.sortScopesForMerge(ScopeResolver.normalizeScopes(scopes));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is hot code... if we want to do this at the very least leave a //TODO: Avoid using ScopeResolver.normalizeScopes and normalize scopes using sorted result

Basically, ScopeResolver.normalizeScopes is O(n^2) but after the sort by dfa.sortScopesForMerge normalization is trivially done in O(n), see:

// At this stage i = n or j = m, meaning that one of our two lists is now
// empty, so we just add everything from one of them. But to ensure
// normalization, we still do the endsWith('*') trick, skipping scopes that
// are already satisfied.
while (i < n) {
let scope = scopes1[i];
scopes.push(scope);
i += 1;
if (scope.endsWith('*')) {
let prefix = scope.slice(0, -1);
while(i < n && scopes1[i].startsWith(prefix)) {
i += 1;
}
}
}

Technically, this could be done with mergeScopeSets(dfa.sortScopesForMerge(_.clone(scopes)), []) which probably isn't even that bad an idea...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm saying this because we have spent time making sure this code path was fast.. :)

@jonasfj
Copy link
Contributor

jonasfj commented Jan 29, 2016

r+ But please do fix the delete to get change :)

@djmitche
Copy link
Contributor Author

Updated -- thanks!

djmitche added a commit to djmitche/taskcluster-auth that referenced this pull request Feb 1, 2016
@djmitche djmitche merged commit 2e86c5c into taskcluster:master Feb 1, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants