Hi, I updated based on the review. I thought that using ABVSpool is similar to using PointablePool. Shall I update instances where nested object keys are not found to just use a new ABVS without getting it from a pool if it would be more memory efficient?
Thank you. Yours sincerely, Riyafa On 10 November 2016 at 01:38, Preston Carman <[email protected]> wrote: > I posted a review. > > The ABVS pool is needed for getting nested JSON object keys. I am just > wondering if all the places that were updated to use the pool, > actually require a ABVS pool. > > On Wed, Nov 9, 2016 at 12:54 AM, Riyafa Abdul Hameed <[email protected]> > wrote: > > Hi, > > > > I have created an ABVS pool based on a previous discussion in one of the > > comments and during the meetings if I am not mistaken. > > > > Thank you. > > > > Yours sincerely, > > Riyafa > > > > On 8 November 2016 at 11:11, Preston Carman <[email protected]> wrote: > > > >> I think this PR needs a review. I did a quick glance at it and found > >> at least one formatting issue. Also, this change needs a ABVS pool, > >> but this structure is only helpful when things are nested. A pool is > >> not required if the function does not deal with a nested result. In > >> practice the pool will be slower than using a single ABVS, if no > >> nesting is in the function. > >> > >> On Sat, Nov 5, 2016 at 10:08 AM, Riyafa Abdul Hameed <[email protected] > > > >> wrote: > >> > Hi, > >> > > >> > Can this PR[1] be merged? > >> > > >> > [1] https://github.com/apache/vxquery/pull/147 > >> > > >> > Thank you. > >> > > >> > Yours sincerely, > >> > Riyafa > >> > > >> > On 27 August 2016 at 15:23, Riyafa Abdul Hameed <[email protected]> > >> wrote: > >> > > >> >> Hi, > >> >> > >> >> I have updated the PR[1] based on the suggestions. > >> >> > >> >> > >> >> [1] https://github.com/apache/vxquery/pull/147 > >> >> Thank you. > >> >> > >> >> Yours sincerely, > >> >> Riyafa > >> >> > >> >> On 21 August 2016 at 22:48, Till Westmann <[email protected]> wrote: > >> >> > >> >>> Hi Riyafa, > >> >>> > >> >>> I looks good. I’ve added 2 small comments. Once those are > addressed, I > >> >>> think > >> >>> that it’s fine to use it in other classes. > >> >>> > >> >>> Cheers, > >> >>> Till > >> >>> > >> >>> > >> >>> On 20 Aug 2016, at 21:13, Riyafa Abdul Hameed wrote: > >> >>> > >> >>> Hi, > >> >>>> > >> >>>> I have created a class as ArrayBackedValueStoragesPool[1]. Please > >> >>>> suggest > >> >>>> if it should be written differently. If it looks good then am I to > >> change > >> >>>> other classes to use it? > >> >>>> > >> >>>> [1] > >> >>>> https://github.com/riyafa/vxquery/blob/1e305db7bc2c17daf7446 > >> >>>> f5c20ebb7a5cb7dba67/vxquery-core/src/main/java/org/apache/vx > >> >>>> query/datamodel/accessors/ArrayBackedValueStoragesPool.java > >> >>>> > >> >>>> Thank you. > >> >>>> > >> >>>> Yours sincerely, > >> >>>> Riyafa > >> >>>> > >> >>>> On 19 August 2016 at 09:29, Till Westmann <[email protected]> > wrote: > >> >>>> > >> >>>> Hi Riyafa, > >> >>>>> > >> >>>>> yes, I think that that would make sense. The idea is that one > would > >> >>>>> have a > >> >>>>> pool of reusable ArrayBackedValueStores. Each of those would > probably > >> >>>>> grow > >> >>>>> for a while up to the maximum size that is needed for the > evaluation > >> of > >> >>>>> an > >> >>>>> operator/function in a plan. And one the maximum size is reached, > the > >> >>>>> stores are re-used until the end of the evaluation without > requiring > >> >>>>> additional allocation or garbage collection. > >> >>>>> > >> >>>>> Cheers, > >> >>>>> Till > >> >>>>> > >> >>>>> > >> >>>>> On 18 Aug 2016, at 18:33, Riyafa Abdul Hameed wrote: > >> >>>>> > >> >>>>> Hi, > >> >>>>> > >> >>>>>> > >> >>>>>> Regarding the suggestions on the PR[1], currently there's only a > >> >>>>>> PointablePooleFactory. Am I to create an > >> >>>>>> ArrayBackedValueStoragesPoolFactory? > >> >>>>>> > >> >>>>>> [1] https://github.com/apache/vxquery/pull/147 > >> >>>>>> > >> >>>>>> Thank you. > >> >>>>>> Yours sincerely, > >> >>>>>> Riyafa > >> >>>>>> > >> >>>>>> > >> >>>>>> On 18 August 2016 at 19:15, Riyafa Abdul Hameed < > >> >>>>>> [email protected]> > >> >>>>>> wrote: > >> >>>>>> > >> >>>>>> Hi, > >> >>>>>> > >> >>>>>>> > >> >>>>>>> I have created a PR[1] to resolve the issue. Please kindly > review. > >> >>>>>>> > >> >>>>>>> Thank you. > >> >>>>>>> > >> >>>>>>> Yours sincerely, > >> >>>>>>> Riyafa > >> >>>>>>> > >> >>>>>>> [1] https://github.com/apache/vxquery/pull/147 > >> >>>>>>> > >> >>>>>>> On 17 August 2016 at 22:17, Riyafa Abdul Hameed < > >> >>>>>>> [email protected] > >> >>>>>>> > >> >>>>>>>> > >> >>>>>>>> wrote: > >> >>>>>>> > >> >>>>>>> Hi, > >> >>>>>>> > >> >>>>>>>> > >> >>>>>>>> As I have commented on the issue VXQUERY-227[1] I would like to > >> know > >> >>>>>>>> what > >> >>>>>>>> the method signature should be? > >> >>>>>>>> > >> >>>>>>>> "Should the implementation of the getkeys method also take an > >> >>>>>>>> IPointable > >> >>>>>>>> as an argument together with the IMutableValueStorage or > should it > >> >>>>>>>> take > >> >>>>>>>> only the IMutableValueStorage as the argument? > >> >>>>>>>> I mean should the implementation be getKeys(IPointable p, > >> >>>>>>>> IMutableValueStorage mvs) or should it be just > >> >>>>>>>> getKeys(IMutableValueStorage > >> >>>>>>>> mvs) ?" > >> >>>>>>>> > >> >>>>>>>> Thank you. > >> >>>>>>>> Yours sincerely, > >> >>>>>>>> Riyafa > >> >>>>>>>> > >> >>>>>>>> [1] https://issues.apache.org/jira/browse/VXQUERY-227?page=com > . > >> >>>>>>>> atlassian.jira.plugin.system.issuetabpanels:comment-tabpane > >> >>>>>>>> l&focusedCommentId=15424494#comment-15424494 > >> >>>>>>>> > >> >>>>>>>> -- > >> >>>>>>>> Riyafa Abdul Hameed > >> >>>>>>>> Undergraduate, University of Moratuwa > >> >>>>>>>> > >> >>>>>>>> Email: [email protected] > >> >>>>>>>> Website: https://riyafa.wordpress.com/ < > >> http://riyafa.wordpress.com/ > >> >>>>>>>> > > >> >>>>>>>> <http://facebook.com/riyafa.ahf> <http://lk.linkedin.com/in/ > >> riyafa> > >> >>>>>>>> <http://twitter.com/Riyafa1> > >> >>>>>>>> > >> >>>>>>>> > >> >>>>>>>> > >> >>>>>>> > >> >>>>>>> -- > >> >>>>>>> Riyafa Abdul Hameed > >> >>>>>>> Undergraduate, University of Moratuwa > >> >>>>>>> > >> >>>>>>> Email: [email protected] > >> >>>>>>> Website: https://riyafa.wordpress.com/ < > >> http://riyafa.wordpress.com/> > >> >>>>>>> <http://facebook.com/riyafa.ahf> <http://lk.linkedin.com/in/ > >> riyafa> > >> >>>>>>> <http://twitter.com/Riyafa1> > >> >>>>>>> > >> >>>>>>> > >> >>>>>>> > >> >>>>>> > >> >>>>>> -- > >> >>>>>> Riyafa Abdul Hameed > >> >>>>>> Undergraduate, University of Moratuwa > >> >>>>>> > >> >>>>>> Email: [email protected] > >> >>>>>> Website: https://riyafa.wordpress.com/ < > >> http://riyafa.wordpress.com/> > >> >>>>>> <http://facebook.com/riyafa.ahf> <http://lk.linkedin.com/in/ > riyafa > >> > > >> >>>>>> <http://twitter.com/Riyafa1> > >> >>>>>> > >> >>>>>> > >> >>>>> > >> >>>> > >> >>>> -- > >> >>>> Riyafa Abdul Hameed > >> >>>> Undergraduate, University of Moratuwa > >> >>>> > >> >>>> Email: [email protected] > >> >>>> Website: https://riyafa.wordpress.com/ < > http://riyafa.wordpress.com/> > >> >>>> <http://facebook.com/riyafa.ahf> <http://lk.linkedin.com/in/ > riyafa> > >> >>>> <http://twitter.com/Riyafa1> > >> >>>> > >> >>> > >> >> > >> > -- Riyafa Abdul Hameed Undergraduate, University of Moratuwa Email: [email protected] Website: https://riyafa.wordpress.com/ <http://riyafa.wordpress.com/> <http://facebook.com/riyafa.ahf> <http://lk.linkedin.com/in/riyafa> <http://twitter.com/Riyafa1>
