> On Jan. 24, 2017, 11:38 p.m., Madhan Neethiraj wrote: > > repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java, > > line 795 > > <https://reviews.apache.org/r/51092/diff/5/?file=1613767#file1613767line795> > > > > Instead of the following: > > AtlasVertex[] result = new AtlasVertex[instancesForClass.size()]; > > // ... > > return Arrays.asList(result); > > > > It might be efficient to use Vector, like: > > Vector<AtlasVertex> result = new Vector<>(); > > result.setSize(instancesForClass.size()); > > // ... > > return result; > > > > Please review.
That is definitely an interesting idea. I'm not sure that would be more efficient, though. In result.setSize(), all of the elements in the Array underlying the Vector need to be explcitly nulled out. We avoid that overhead by making the result be an array initially. In addition, Arrays.asList() is very efficient. It creates a List that directly uses the array provided, without doing any copying. I actually doubt there would be much difference. I can change it if you feel strongly about this. > On Jan. 24, 2017, 11:38 p.m., Madhan Neethiraj wrote: > > repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java, > > line 803 > > <https://reviews.apache.org/r/51092/diff/5/?file=1613767#file1613767line803> > > > > This would work only if all entries in 'instancesForClass' list have > > typeName == classType.getName(). If the list can have subType of > > classType.getName() - this query will skip these instances. Perhaps this is > > intentional - if yes, please ignore. I made the check this way because that is what the original getVertexForInstanceByUniqueAttribute() did. I did not want to change the behavior. result = findVertex(propertyKey, instance.get(attributeInfo.name), Constants.ENTITY_TYPE_PROPERTY_KEY, classType.getName(), Constants.STATE_PROPERTY_KEY, Id.EntityState.ACTIVE.name()); > On Jan. 24, 2017, 11:38 p.m., Madhan Neethiraj wrote: > > repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java, > > line 794 > > <https://reviews.apache.org/r/51092/diff/5/?file=1613767#file1613767line794> > > > > Consider moving this block to start of the method, to avoid unused > > "map" that gets populated above. I'm not sure I understand your comment. Are you suggesting that we should loop through the attributes twice in order to avoid creating the map if there are no unique attributes (once to determine if there are any unique attributes, and a second time to populate "map")? The way it is now, if there are no unique attribues, nothing will be added to "map". > On Jan. 24, 2017, 11:38 p.m., Madhan Neethiraj wrote: > > repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java, > > line 566 > > <https://reviews.apache.org/r/51092/diff/5/?file=1613767#file1613767line566> > > > > Collection<Integer> as the map value is needed only to handle duplicate > > values in 'values'. If most of the queries are expected to have > > non-duplicate values, avoiding this overhead (in creating a number of list > > objects, ..), especially at this lower level code, could help improve > > performance. > > > > One approach could be to use Map<String, AtlasVertex> as the return > > type from this method. This approach could eliminate the need for > > NonExistentVertexHandling flag. Please review. Great suggestion. - Jeff ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51092/#review162863 ----------------------------------------------------------- On Jan. 24, 2017, 11:22 p.m., Jeff Hagelberg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51092/ > ----------------------------------------------------------- > > (Updated Jan. 24, 2017, 11:22 p.m.) > > > Review request for atlas and David Kantor. > > > Repository: atlas > > > Description > ------- > > Apply performance fixes for create/update entities from IBM fork to Atlas. > During our performance profiling, we found a number of performance hotspots > in JProfiler. Our main findings were > > - multiple queries were being executed for each instance being > created/updated to find matches by unique attribute. > - one query was being executed for each instance being created/updated to > find the corresponding vertex if there is one > - Calculating the value of the full text property was taking a > significant portion of the time to create/update entities, mainly due to its > calls to getVertexForGUID > > The changes we put in do the following: > > - batch lookups by guid when create/update entities. Execute one > AtlasGraphQuery to find them all. > - batch lookups by unique attribute when create/update entities. Execute > one AtlasGraphQuery per class to find unique attribute matches. > - find all existing vertices up front during create/update entity. Use > those vertices during the graph mapping process to avoid running unnecessary > graph queries > - reuse reference vertices from instance to graph mapping when computing > full text property > > Also, resolved all test failures in webapp. I disentagled the three > competing versions of the hive model that the various tests were trying to > use. Now they all pass. I tried to follow the path of least resistence. We > really should clean this up more, there is really no need for threee > different versions of hive_table and its related classes. > > > Diffs > ----- > > > repository/src/main/java/org/apache/atlas/repository/audit/InMemoryEntityAuditRepository.java > 50a007bf1bf7e8c07a57b327b3aa3b907dd5f660 > > repository/src/main/java/org/apache/atlas/repository/graph/FullTextMapper.java > 911b1adbad92a76ce15f49ce023b56aeca8b4f94 > > repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepository.java > 0c80aeddbae35c80cdf8b83cea6aefadf6454a20 > repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java > 889236ca805142a93d9d4e63789fb0cc9aea05aa > > repository/src/main/java/org/apache/atlas/repository/graph/TypedInstanceToGraphMapper.java > 4e55bbcab91b8572d13345cec61e8df2f195ee4f > > repository/src/main/java/org/apache/atlas/repository/graph/VertexLookupContext.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/services/DefaultMetadataService.java > 35a489f2d77578a72a2e73a37bdf094af25a166e > repository/src/main/java/org/apache/atlas/util/AttributeValueMap.java > PRE-CREATION > repository/src/main/java/org/apache/atlas/util/IndexedInstance.java > PRE-CREATION > > repository/src/test/java/org/apache/atlas/repository/graph/GraphHelperTest.java > a7dc13db72fb4ff268312c106df1b6c41f46962f > typesystem/src/test/resources/atlas-application.properties > 108630b485e712179dc80d001dbce97551b37516 > webapp/src/main/java/org/apache/atlas/web/resources/EntityResource.java > 17c8237569746bb77f75692d50ce115c21b80c7c > > webapp/src/test/java/org/apache/atlas/notification/EntityNotificationIT.java > 1774611285956a7f187bd4353778fe34a6893c69 > > webapp/src/test/java/org/apache/atlas/notification/NotificationHookConsumerIT.java > 4a3db8874468bc8373625afe93df0a0938d00e39 > webapp/src/test/java/org/apache/atlas/web/resources/BaseResourceIT.java > 51be64c0dccecd56e720f3633ba46bbaf5c37f5d > > webapp/src/test/java/org/apache/atlas/web/resources/DataSetLineageJerseyResourceIT.java > 8334e4f9dd2eedf48e516ee34a5b2981489ce0da > > webapp/src/test/java/org/apache/atlas/web/resources/EntityDiscoveryJerseyResourceIT.java > 2bbe10a0827b2035a6c74e6b3aa3520b7e12d571 > > webapp/src/test/java/org/apache/atlas/web/resources/EntityJerseyResourceIT.java > f084053a03f9940d2f9015d4a15ef9c085553ae5 > > webapp/src/test/java/org/apache/atlas/web/resources/EntityV2JerseyResourceIT.java > 74338fd7aee2d81f54f59b0be15bd249852fbd0b > > webapp/src/test/java/org/apache/atlas/web/resources/MetadataDiscoveryJerseyResourceIT.java > b004cb52cc996763dbc4c24cd80ab545c5749358 > > Diff: https://reviews.apache.org/r/51092/diff/ > > > Testing > ------- > > Ran complete build on linux, all tests passed > > > Thanks, > > Jeff Hagelberg > >
