Uploaded image for project: 'Spring Data Commons'
  1. Spring Data Commons
  2. DATACMNS-1102

Avoid superfluous recreation of DefaultConversionService in ResultProcessor

    Details

      Description

      When measuring JPQL/Criteria API vs Spring Data JPA projection execution time, Spring Data JPA is way slower than the other.

      Avg for JPQL/Criteria API: 73ms
      Avg for Spring Data JPA: 774ms

      The slowness comes from reinstantiating the DefaultConversionService in ProjectingConverter. Maybe using the shared instance from DefaultConversionService would be a solution for it to eliminate the cost of reinstantiation, see the discussion with Oliver Gierke on Twitter.

        Activity

        Hide
        olivergierke Oliver Gierke added a comment -

        This is in place now. Feel free to give the snapshots a try.

        Show
        olivergierke Oliver Gierke added a comment - This is in place now. Feel free to give the snapshots a try.
        Hide
        galovics Arnold Galovics added a comment - - edited

        Just gave it a try.

        It seems that the same is happening in the constructor of ProjectingMethodInterceptor.

        http://prntscr.com/frm2vo

        Show
        galovics Arnold Galovics added a comment - - edited Just gave it a try. It seems that the same is happening in the constructor of ProjectingMethodInterceptor . http://prntscr.com/frm2vo
        Hide
        galovics Arnold Galovics added a comment -

        Hi @olivergierke,

        The new snapshot version seems to be OK.

        Fetching 1000 projections via the repository is executed within 180ms which is still slower than using Criteria API/JPQL projection by a factor of 3 but of course, there will be overhead.

        Maybe we might analyze what's happening which causes 3 times slower execution in the context of another ticket. What do you think?

        Show
        galovics Arnold Galovics added a comment - Hi @olivergierke, The new snapshot version seems to be OK. Fetching 1000 projections via the repository is executed within 180ms which is still slower than using Criteria API/JPQL projection by a factor of 3 but of course, there will be overhead. Maybe we might analyze what's happening which causes 3 times slower execution in the context of another ticket. What do you think?
        Hide
        olivergierke Oliver Gierke added a comment -

        Yes, please make sure we back those tickets with proper JMH benchmarks. Especially when databases get involved, there are tons of things that can create additional overhead and/or distort results as e.g. with an in-memory DB, the actual object lookup is basically a Map lookup and makes the surrounding execution look expensive which in a real world scenario, when you read from a real database, it's neglectable. The projections and the repository API in general add quite a bit of convenience but of course there is no free lunch and the by definition will be some overhead.

        Oh, please don't tweak the ticket status at will as we use them for internal project management quite a bit.

        Show
        olivergierke Oliver Gierke added a comment - Yes, please make sure we back those tickets with proper JMH benchmarks. Especially when databases get involved, there are tons of things that can create additional overhead and/or distort results as e.g. with an in-memory DB, the actual object lookup is basically a Map lookup and makes the surrounding execution look expensive which in a real world scenario, when you read from a real database, it's neglectable. The projections and the repository API in general add quite a bit of convenience but of course there is no free lunch and the by definition will be some overhead. Oh, please don't tweak the ticket status at will as we use them for internal project management quite a bit.
        Hide
        quaff zhouyanming added a comment -

        Why not use DefaultConversionService.getSharedInstance() instead of new DefaultConversionService() ?

        Show
        quaff zhouyanming added a comment - Why not use DefaultConversionService.getSharedInstance() instead of new DefaultConversionService() ?
        Hide
        olivergierke Oliver Gierke added a comment -

        Because we want to avoid recreation of the service, but at the same time not share it with anybody else as that means we don't know what additional converters might have been registered.

        Show
        olivergierke Oliver Gierke added a comment - Because we want to avoid recreation of the service, but at the same time not share it with anybody else as that means we don't know what additional converters might have been registered.

          People

          • Assignee:
            olivergierke Oliver Gierke
            Reporter:
            galovics Arnold Galovics
            Last updater:
            Oliver Gierke
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: