Project

General

Profile

bug #9422

Improve performance during Preferences startup

Added by Andreas Müller 5 months ago. Updated 5 months ago.

Status:
Closed
Priority:
Highest
Assignee:
Category:
taxeditor
Target version:
Start date:
01/25/2021
Due date:
% Done:

0%

Severity:
critical
Found in Version:
Tags:

Description

During startup the following function is called

CdmPreferenceCache.getAllTaxEditorDBPreferences by PreferenceUtil.updateDBPreferences.

This code loads the terms very unefficiently if terms are explicitly defined in the local or DB preferences by calling

              List<DefinedTermBase> definedTermBases = termService.load(uuidList, null);
                List<TermDto> dtos = new ArrayList<>();
                for (DefinedTermBase<?> term: definedTermBases){
                    dtos.add(TermDto.fromTerm(term));
                }

So there is 1 or more sebservice call(s) for EACH term as dtos.add requires lazy loading.

A simple solution could be to at least load all DTOs serverside within 1 service call for all termtypes or atleast for each termtype.

Another more dangerous solution could be to use property path, but this is error prone if things change or if a term types needs more loading then defined in the property path.

An even better solution (maybe for the future/another ticket) could be to load the preferences asynchronously and / or using a persisted cache via ehcache. However, these solutions are more complex.

===

In general we should find similar such issues e.g. by switching on CachingHttpInvokerProxyFactoryBean.measureDuration (by using VMarg -Dremoting.httpinvoker.measureDuration=true and setting logging to CachingHttpInvokerProxyFactoryBean to info) which shows you each httpinvoker call and its duration. During development this (or a similar functionality should always be used !!).

===

A last issue:

for CdmPreferenceCache.getAllTaxEditorDBPreferences() the code

        PrefKey key = CdmPreference.NewKey(PreferenceSubject.NewTaxEditorInstance(), PreferencePredicate.AvailableDistributionStatus);

        if (get(key) != null){
            if (!PreferencesUtil.getOverrideForPreference(PreferencePredicate.AvailableDistributionStatus.getKey()) || !get(key).isAllowOverride()){
                //get terms for the uuids... and add them to the termManager as preferred terms
                ITermService termService = CdmStore.getService(ITermService.class);
                List<UUID> uuidList = new ArrayList<>();
                if (get(key).getValue() != null){
                    String[] uuidArray =findBestMatching(key).getValue().split(";");
                    for (String uuidString:uuidArray){
                        try {
                            uuidList.add(UUID.fromString(uuidString));
                        } catch (Exception e) {
                            logger.warn("Preference loading failed", e);
                        }
                    }
                }

                List<DefinedTermBase> definedTermBases = termService.load(uuidList, null);
                List<TermDto> dtos = new ArrayList<>();
                for (DefinedTermBase<?> term: definedTermBases){
                    dtos.add(TermDto.fromTerm(term));
                }
                CdmStore.getTermManager().setPreferredTermsByType(dtos, TermType.PresenceAbsenceTerm);
            }
        }

is very redundantly used 3x. This should be deduplicated. Such c&p within the same class almost always shows that code deduplication should be applied. We should try to avoid this in future.


Related issues

Related to Edit - bug #9423: Improve language handling for TermType.getMessage() and others New 01/25/2021
Related to Edit - feature request #9427: Code cleaning in CdmPreferenceCache Closed 01/28/2021
Related to Edit - feature request #9428: Asynchronous loading of terms New 01/28/2021

Associated revisions

Revision f8326308 (diff)
Added by Katja Luther 5 months ago

ref #9422: get termDtos from server

History

#1 Updated by Andreas Müller 5 months ago

  • Priority changed from New to Highest

#2 Updated by Andreas Müller 5 months ago

  • Related to bug #9423: Improve language handling for TermType.getMessage() and others added

#3 Updated by Andreas Müller 5 months ago

  • Description updated (diff)

#4 Updated by Katja Luther 5 months ago

  • Status changed from New to In Progress

Now the method to get dtos from serverside is used instead of getting the terms and create dtos on editor side.

#5 Updated by Andreas Müller 5 months ago

  • Status changed from In Progress to Feedback
  • Target version changed from Release 5.21 to Release 5.19

The first fix went into 5.19, correct?

So I think we should split the ticket and set this one to resolved?

#6 Updated by Katja Luther 5 months ago

  • Status changed from Feedback to Closed

yes, the first fix (term loading at start) is fixed for 5.19. So I create a new ticket for the remaining issues and close this ticket.

#7 Updated by Katja Luther 5 months ago

#8 Updated by Katja Luther 5 months ago

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 40 MB)