----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68101/#review206640 -----------------------------------------------------------
agents-common/src/main/java/org/apache/ranger/plugin/util/RangerResourceTrie.java Lines 63 (patched) <https://reviews.apache.org/r/68101/#comment289700> Please check intentation - here and rest of the new code in this patch. agents-common/src/main/java/org/apache/ranger/plugin/util/RangerResourceTrie.java Lines 226 (patched) <https://reviews.apache.org/r/68101/#comment289704> This would create new string for each call to getEvaluatorsForResource() when optIgnoreCase=true. It will help to avoid this string creation. agents-common/src/main/java/org/apache/ranger/plugin/util/RangerResourceTrie.java Lines 241-243 (patched) <https://reviews.apache.org/r/68101/#comment289709> Consider replacing #223 to #255 with: TrieNode<T> curr = root; final int len = resource.length(); int i = 0; while (i < len) { final TrieNode<T> child = curr.getChild(getLookupChar(resource.charAt(i))); if (child == null) { break; } final String childStr = child.getStr(); if (!lookupResource.regionMatches(optIgnoreCase, i, childStr, 0, childStr.length())) { break; } curr = child; i += childStr.length(); } List<T> ret = i == len ? curr.getEvaluators() ? curr.getWildcardEvaluators(); agents-common/src/main/java/org/apache/ranger/plugin/util/RangerResourceTrie.java Lines 463 (patched) <https://reviews.apache.org/r/68101/#comment289701> For easier readability, consider replacing: lines #463 and #475 with - newChild.addChild(child); lines #461 and #473 with - this.addChild(newChild) class TrieNode private void addChild(TrieNode child) { children.put(child.getStr().charAt(0), child); } } agents-common/src/main/java/org/apache/ranger/plugin/util/RangerResourceTrie.java Line 454 (original), 529 (patched) <https://reviews.apache.org/r/68101/#comment289702> Having !isSharingParentWildcardEvaluators in 'if' avoids shared list from getting sorted multiple times. Why remove this? agents-common/src/main/java/org/apache/ranger/plugin/util/RangerResourceTrie.java Line 458 (original), 533 (patched) <https://reviews.apache.org/r/68101/#comment289703> Having 'evaluators != wildcardEvaluators' in 'if' avoids duplicate sorting, when evaluator and wildcardEvaluators are the same list. Why remove this? - Madhan Neethiraj On July 31, 2018, 2:50 a.m., Abhay Kulkarni wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68101/ > ----------------------------------------------------------- > > (Updated July 31, 2018, 2:50 a.m.) > > > Review request for ranger, Madhan Neethiraj, Mehul Parikh, Pradeep Agrawal, > Ramesh Mani, and Velmurugan Periasamy. > > > Bugs: RANGER-2173 > https://issues.apache.org/jira/browse/RANGER-2173 > > > Repository: ranger > > > Description > ------- > > Ranger uses Trie data structure to look up policy resources for efficient > access. Trie tree may be optimized to contain fewer nodes and can be made > less deep. This will allow faster construction of Trie tree and faster lookup > for a resource. This is done by using longest common prefix strings instead > of single character to organize Trie tree. Also added instrumentation to > measure performance. > > > Diffs > ----- > > > agents-common/src/main/java/org/apache/ranger/plugin/util/RangerResourceTrie.java > e7e8cf519 > agents-common/src/test/resources/log4j.xml d1a6f1c8a > > > Diff: https://reviews.apache.org/r/68101/diff/2/ > > > Testing > ------- > > Passes all unit tests > > > Thanks, > > Abhay Kulkarni > >
