Hi Riyafa, I’ve checked that Preston’s comments were addressed merged the change. I think that further improvements can be made in a new change.
Cheers, Till On 10 Nov 2016, at 6:24, Riyafa Abdul Hameed wrote: > 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>
