View Issue Details

IDProjectCategoryView StatusLast Update
0004217FreeCADBugpublic2021-12-20 01:43
Reporterapelleti Assigned Tochennes  
PrioritynormalSeveritytweakReproducibilityN/A
Status confirmedResolutionopen 
Target Version0.20 
Summary0004217: Simple perf improvement: avoid 'if somekey in mydict.keys()'
DescriptionThis ticket is a suggestion to fix some very simple (common) coding errors that can seriously degrade performance in python.

Doing a python dictionary hash lookup the recommended way:

    # test if 'somekey' is in dictionary mydict. order-1
    if "somekey" in mydict:
        do_something()

Doing a python dictionary hash lookup the wrong way:

    # walk the dictionary to compile an unsorted list of keys: order-N,
    # then brute force search for desired key in this unsorted set: order-N
    if "somekey" in mydict.keys():
        do_something()

======

Performance comparison:

# good way:
import random
mydict={k: random.random() for k in range(200000)}
for i in mydict:
    if i not in mydict:
        print("FAIL!")
Runtime=0.07 seconds to test 200k keys exist in the dictionary of 200k entries.

----------

# bad way:
import random
mydict={k: random.random() for k in range(200000)}
for i in mydict:
    if i not in mydict.keys():
        print("FAIL!")
Runtime=11min 29sec.....10000 times slower!! :-(

See "additional info" for a partial list.
Steps To ReproduceN/A
Additional InformationHere's a partial list of potential fixes, using a rudimentary grep to search the codebase for occurrences.

DISCLAIMER: likely a few false-positives.

[dhcp-161-44-212-77:~/Downloads/FreeCAD-master] apelleti% find . -name "*.py" | xargs egrep "if.* in .*keys" 
./src/Tools/MakeMacBundleRelocatable.py:                    if dk not in visited.keys():
./src/Tools/generateBase/generateDS.py:        if 'maxOccurs' in self.attrs.keys():
./src/Tools/generateBase/generateDS.py:        if 'mixed' in self.attrs.keys():
./src/Tools/generateBase/generateDS.py:            if 'substitutionGroup' in attrs.keys()and 'name' in attrs.keys():
./src/Tools/generateBase/generateDS.py:            if 'name' in attrs.keys():
./src/Tools/generateBase/generateDS.py:            elif 'ref' in attrs.keys():
./src/Tools/generateBase/generateDS.py:            if 'type' in attrs.keys():
./src/Tools/generateBase/generateDS.py:            if 'use' in attrs.keys():
./src/Tools/generateBase/generateDS.py:            if 'name' in attrs.keys():
./src/Tools/generateBase/generateDS.py:            if 'ref' in attrs.keys():
./src/Tools/generateBase/generateDS.py:            if 'base' in attrs.keys() and len(self.stack) > 0:
./src/Mod/Draft/Draft.py:            if str(obj.ViewObject.Pattern) in list(svgpatterns().keys()):
./src/Mod/Draft/Draft.py:                            if str(vobj.Pattern) in list(svgpatterns().keys()):
./src/Mod/Drawing/DrawingPatterns.py:    if not (name in Patterns.keys()):
./src/Mod/Drawing/DrawingPatterns.py:    if not (name in Patterns.keys()):
./src/Mod/Drawing/DrawingPatterns.py:    if not (name in Patterns.keys()):
./src/Mod/Spreadsheet/App/Spreadsheet_legacy.py:                if key in self._cells.keys():
./src/Mod/Spreadsheet/App/Spreadsheet_legacy.py:            if "Type" in self._cells.keys():
./src/Mod/Spreadsheet/App/Spreadsheet_legacy.py:            if "Object" in self._cells.keys():
./src/Mod/Spreadsheet/App/Spreadsheet_legacy.py:                if key in self.spreadsheet.Proxy._cells.keys():
./src/Mod/Spreadsheet/App/Spreadsheet_legacy.py:                if key in obj.Proxy._cells.keys():
./src/Mod/Path/PathScripts/PathPost.py:        if item.text() in self.tooltips.keys():
./src/Mod/Path/PathScripts/PathPreferencesPathJob.py:        if not name in self.processor.keys():
./src/Mod/Path/PathScripts/PathToolLibraryManager.py:        if target in tt.Tools.keys():
./src/Mod/Path/PathScripts/PathToolLibraryManager.py:        if target in tt.Tools.keys():
./src/Mod/Idf/Idf.py:        if len(model_records)>1 and model_records[0] and not model_records[0] in keys:
./src/Mod/Arch/importDAE.py:                                if "target" in mnode.keys():
./src/Mod/Arch/ArchReference.py:            if self.obj.Part in parts.keys():
./src/Mod/Arch/ArchReference.py:                    if self.obj.Part in parts.keys():
./src/Mod/Arch/exportIFC.py:            if "IfcUID" in obj.IfcData.keys():
./src/Mod/Arch/exportIFC.py:        if ifctype not in ArchIFCSchema.IfcProducts.keys():
./src/Mod/Arch/exportIFC.py:            if "FlagForceBrep" in obj.IfcData.keys():
./src/Mod/Arch/exportIFC.py:                    if c.Name in products.keys():
./src/Mod/Arch/exportIFC.py:                        if c.Name in products.keys():
./src/Mod/Arch/exportIFC.py:                if c.Name in products.keys():
./src/Mod/Arch/exportIFC.py:            if g[0] in groups.keys():
./src/Mod/Arch/exportIFC.py:                if o in products.keys():
./src/Mod/Arch/exportIFC.py:                elif o in annos.keys():
./src/Mod/Arch/exportIFC.py:    if ifctype in translationtable.keys():
./src/Mod/Arch/ArchCommands.py:        if "FlagForceBrep" in d.keys():
./src/Mod/Arch/ArchComponent.py:        self.psetkeys = [''.join(map(lambda x: x if x.islower() else " "+x, t[5:]))[1:] for t in self.psetdefs.keys()]
./src/Mod/Arch/ArchIFC.py:IfcTypes = [''.join(map(lambda x: x if x.islower() else " "+x, t[3:]))[1:] for t in ArchIFCSchema.IfcProducts.keys()]
./src/Mod/Arch/ArchIFC.py:            if attribute["name"] not in ifcComplexAttributes.keys():
./src/Mod/Arch/ArchIFC.py:        return [''.join(map(lambda x: x if x.islower() else " "+x, t[3:]))[1:] for t in schema.keys()]
./src/Mod/Arch/importIFClegacy.py:            if "FlagForceBrep" in obj.IfcAttributes.keys():
./src/Mod/Arch/importSH3D.py:            if "angle" in attributes.keys():
./src/Mod/Arch/importSH3D.py:                if "angle" in attributes.keys():
./src/Mod/Arch/importSH3D.py:                if "elevation" in attributes.keys():
./src/Mod/Arch/ArchIFCView.py:            if lineEdit.objectName() in data.keys():
./src/Mod/Arch/importIFC.py:            if currentid in additions.keys():
./src/Mod/Arch/importIFC.py:                    if layer_name not in list(layers.keys()):
./src/Mod/Arch/importIFC.py:            if "FreeCADPropertySet" in [ifcfile[pset].Name for pset in psets.keys()]:
./src/Mod/Arch/importIFC.py:                    if c in structshapes.keys():
./src/Mod/Arch/importIFC.py:                if host in objects.keys():
./src/Mod/Arch/importIFC.py:                        if child in objects.keys():
./src/Mod/Arch/importIFC.py:                if child in objects.keys():
./src/Mod/Arch/importIFC.py:                    if c in shapes.keys():
./src/Mod/Arch/importIFC.py:                    if c in additions.keys():
./src/Mod/Arch/importIFC.py:                            if c2 in shapes.keys():
./src/Mod/Arch/importIFC.py:                if (subtraction[0] in objects.keys()) and (subtraction[1] in objects.keys()):
./src/Mod/Arch/importIFC.py:            if host not in objects.keys():
./src/Mod/Arch/importIFC.py:                if child in objects.keys() \
./src/Mod/Arch/importIFC.py:            if aid in remaining.keys():
./src/Mod/Arch/importIFC.py:                    if (aid in children) and (host in objects.keys()):
./src/Mod/Arch/importIFC.py:        if "FreeCADType" in appset.keys():
./src/Mod/Arch/importIFC.py:            if "FreeCADName" in appset.keys():
./src/Mod/AddonManager/addonmanager_workers.py:                            if not wb.strip() in FreeCADGui.listWorkbenches().keys():
./src/Mod/AddonManager/addonmanager_workers.py:                                if not wb.strip()+"Workbench" in FreeCADGui.listWorkbenches().keys():
./src/Mod/PartDesign/FeatureHole/Standards.py:    if not standard in standards.keys():
./src/Mod/PartDesign/FeatureHole/Standards.py:    if not standard in standards_through.keys():
./src/Mod/PartDesign/FeatureHole/Standards.py:    if not standard in standards_counterbore_through.keys():
./src/Mod/PartDesign/FeatureHole/Standards.py:    if not standard in standards_counterbore.keys():
./src/Mod/PartDesign/FeatureHole/Standards.py:    if not standard in standards_counterbore_rows.keys():
./src/Mod/PartDesign/FeatureHole/Standards.py:    if not standard in standards_countersink.keys():
./src/Mod/PartDesign/FeatureHole/Standards.py:    if not standard in standards_countersink.keys():
./src/Mod/PartDesign/FeatureHole/Standards.py:    if not standard in standards_thread.keys():
./src/Mod/PartDesign/FeatureHole/Standards.py:    if not standard in standards_threaded.keys():
./src/Mod/Material/importFCMat.py:                if k in group.keys():
./src/Mod/Material/materialtools/cardutils.py:            if k in registed_cardkeys:
./src/Mod/Material/materialtools/cardutils.py:        if (k not in used_and_registered_cardkeys) and (k not in used_and_not_registered_cardkeys):
TagsNo tags attached.
FreeCAD InformationOS: macOS 10.14
Word size of OS: 64-bit
Word size of FreeCAD: 64-bit
Version: 0.18.16146 (Git)
Build type: Release
Branch: (HEAD detached at 0.18.4)
Hash: 980bf9060e28555fecd9e3462f68ca74007b70f8
Python version: 3.6.7
Qt version: 5.6.2
Coin version: 4.0.0a
OCC version: 7.3.0
Locale: English/Canada (en_CA)

Activities

openBrain

2019-12-06 11:01

developer   ~0013877

@apelleti : please open a forum topic and cross-link with this ticket (per the guidelines in the huge yellow box on the top of the page).
BTW : your command line will be better written like
find . -name "*.py" -exec egrep -Hn "if.* in .*\.keys\(\)" {} \;

Kunda1

2020-01-14 18:14

administrator   ~0014074

Last edited: 2020-01-14 20:02

Forum thread https://forum.freecadweb.org/viewtopic.php?f=3&t=41483

chennes

2021-12-17 05:03

administrator   ~0016110

These issues do exist, and simply represent a misunderstanding on the part of the original programmer on what exactly they are asking Python to do. With the exception of the false positives (e.g. where a container's name contains the word "keys") these can all be replaced wholesale without concern that we are breaking something. Best to remove all of them, so later copy-and-paste programmers don't inherit the performance penalty.

Issue History

Date Modified Username Field Change
2019-12-06 02:56 apelleti New Issue
2019-12-06 11:00 openBrain Tag Attached: #post-to-forum
2019-12-06 11:01 openBrain Status new => feedback
2019-12-06 11:01 openBrain Category General => Patch
2019-12-06 11:01 openBrain Note Added: 0013877
2020-01-14 18:14 Kunda1 Note Added: 0014074
2020-01-14 18:14 Kunda1 Tag Detached: #post-to-forum
2020-01-14 18:14 Kunda1 Tag Attached: #pending-forum
2020-01-14 20:02 Kunda1 Note Edited: 0014074
2021-02-06 06:49 abdullah Target Version => 0.20
2021-12-17 05:03 chennes Assigned To => chennes
2021-12-17 05:03 chennes Status feedback => confirmed
2021-12-17 05:03 chennes Note Added: 0016110
2021-12-17 05:04 chennes Tag Detached: #pending-forum
2021-12-17 05:04 chennes Category Patch => Bug
2021-12-20 01:43 chennes Description Updated
2021-12-20 01:43 chennes Additional Information Updated