> On Sept. 1, 2017, 5:01 p.m., Sergio Pena wrote: > > The patch looks good. But just wonder, how does this patch have a memory > > improvement? I see the big change is just to return an ArrayList instead of > > a HashSet in the getPathStrings(). How does this make us better?
Hi Sergio. HashSets use a lot more memory than ArrayLists because of the difference in their internal design. If you are interested, I can explain more, but for now take a look at the small test below. It creates the same number of HashSets and then ArrayLists, with the same workload, and measures how much memory they take each time. Here are the results: $ java TestMem Used memory by sets: 1240 MB Used memory by lists: 156 MB Here is the code: import java.util.ArrayList; import java.util.HashSet; public class TestMem { public static final int NUM_OBJS = 1000 * 1000; public static final int NUM_STRS = 30; public static final String[] STRINGS = new String[NUM_STRS]; public static void main(String args[]) { // Fill the strings array once for (int i = 0; i < NUM_STRS; i++) { STRINGS[i] = Integer.toString(i); } Object[] listsOrSets = new Object[NUM_OBJS]; System.gc(); for (int i = 0; i < NUM_OBJS; i++) { HashSet<String> set = new HashSet<>(NUM_STRS); for (int j = 0; j < NUM_STRS; j++) { set.add(STRINGS[j]); } listsOrSets[i] = set; } reportMemory("sets"); System.gc(); for (int i = 0; i < NUM_OBJS; i++) { ArrayList<String> list = new ArrayList<>(NUM_STRS); for (int j = 0; j < NUM_STRS; j++) { list.add(STRINGS[j]); } listsOrSets[i] = list; } reportMemory("lists"); } private static void reportMemory(String s) { System.gc(); Runtime r = Runtime.getRuntime(); long usedMemInMB = (r.totalMemory() - r.freeMemory()) / 1024 / 1024; System.out.println("Used memory by " + s + ": " + usedMemInMB + " MB"); } } - Misha ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62007/#review184366 ----------------------------------------------------------- On Aug. 31, 2017, 4:14 p.m., Alexander Kolbasov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62007/ > ----------------------------------------------------------- > > (Updated Aug. 31, 2017, 4:14 p.m.) > > > Review request for sentry, Arjun Mishra, Brian Towles, Misha Dmitriev, Na Li, > Sergio Pena, Vamsee Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1909 > https://issues.apache.org/jira/browse/SENTRY-1909 > > > Repository: sentry > > > Description > ------- > > SENTRY-1909 Improvements for memory usage when full path snapshot is sent > from Sentry to NN > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java > 898c7bec2e35e6f1424478666282ba78222da709 > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestImageRetriever.java > 20b3e108cc976207a3809bc6a214a34e10788200 > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestSentryHDFSServiceProcessor.java > e403d7cca5dca5c39785490f92e562dc9a3b4daf > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java > fee5279a7cd3d60df4e60a5f25d2e8604c37d04b > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PathsImage.java > 409a557fe89d85df888c1f16b3f5aacdd9aa96b0 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 593b92f96b47f959266280bce3d0809f6a80c362 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java > 760a2b55e98ac1fb305666cbbedfba11d26f2fcc > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/FullUpdateModifier.java > 2cd18ea9d2ca40b5f46aaafde532fb3bf9769c07 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java > 4a8fb953ce486e1aeb1042884de56872b5539cd0 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > 3da6a4e998d36cada7e9ea77285b11f07cea5f25 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestFullUpdateInitializer.java > 9d7bddd339513180e627286d8a749b7814c824f2 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestFullUpdateModifier.java > 7deccb064fd75b39af779796d9e95caf9c718b32 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java > f56384a99e128b3e9880cbc5692107d61f2f500f > > > Diff: https://reviews.apache.org/r/62007/diff/3/ > > > Testing > ------- > > > Thanks, > > Alexander Kolbasov > >