Skip to content

Commit

Permalink
Fix bug for descriptive stats with huge dataset (400 Gb)
Browse files Browse the repository at this point in the history
Diagnostique etabli grace aux trace et l'analyse gdb de la pile d'appel et des variables sur la machine d'Elias
- les dictionnaires deviennent top gros
- cela entraine un retaillage de la table de hashage s'il y a trop de valeurs distinctes
  - ce retaillage, non securise actuelement, entraine un depassemennt de la limite des entiers (2^31)
  - la taille devient negative, ce qui cree le bug de segementation fault

Bug difficile a reproduire
- il faut une base enorme
- il faut une machine enorme, avec suffisament de RAM pour charger tous les objets d'une seule variable simultanement
- il faut un decoupage de la base en tranche, avec
  - au moins une variable avec enormement de valeurs distinctes (par exemple un identifiant)
  - d'autres variable ayant de nombreuses valeurs distinctes
    (mais pas trop, sionon ce la force un redecoupage de la tranche en sous-tranche plus petites, sans le probleme)

Bug reproduit
- base avec 350 000 000 d'instances
- une variable identifiant et 11 variable avec 10% de valeurs distinctes
    Dictionary  HugeDataSet
    {
    Unused      Categorical     Empty           ;
        Categorical     V1 = Concat("V1_", AsCategorical(Index()));
        Categorical     V2 = IfC(LE(Random(), 0.1), Concat("V2_", AsCategorical(Index())), "");
        ...
        Categorical     V12 = IfC(LE(Random(), 0.1), Concat("V12_", AsCategorical(Index())), "");
    };
- Machine avec beaucoup de RAM, en utilisant un seul processeur
    AnalysisSpec.SystemParameters.MaxCoreNumber 1     // Max number of processor cores
    AnalysisSpec.SystemParameters.MemoryLimit 190000    // Memory limit in MB

Correction
- Obdic.cpp
  - nouvelle entree dans nDictionaryPrimeSizes, au dela de 1 milliard
  - DictionaryGetNextTableSize: rend INT_MAX dans le pire des cas
- retaillage dynamique des dictionnaires
  - uniquement si la taiile n'est pas deja INT_MAX
  - protection contre depassement arithmetique par
    DictionaryGetNextTableSize(2 * min(GetHashTableSize(), INT_MAX / 2)))
  - impacts dans:
    - ObjectDictionary::operator[]
    - NumericKeyDictionary::operator[]
    - KWCDUniqueStringDictionary::AsUniqueString
    - KWSymbolDictionary::AsSymbol
  - refactoring de KWDataGridMerger
    - supression de nCellDictionaryPrimeSizes et CellDictionaryGetNextTableSize
    - reutilisation de DictionaryGetNextTableSize

Autres modifications, hors correction du bug
- Report de corrections effectuees dans la branche dev-v11
  - tolerance sur la gestion des typicalites negatives
  - impacts
    - CCCoclusteringReport::ReadComposition
    - CCCoclusteringReport::ReadJSONValueGroups
- Correction dans la librairie KhiopsNativeInterface pour le chargement des drivers de fichiers
  - methode KNICreateEnv: ajout du not dans "if (not SystemFileDriverCreator::IsExternalDriversRegistered())"
- Supression des fichies KWDRTokenCount.h et .cpp qui etaient en trop (qui contiennent des classes existantes dans d'autres fichiers)
  • Loading branch information
marcboulle committed Oct 9, 2023
1 parent ebcd607 commit 49aaec1
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 1,947 deletions.
1,568 changes: 0 additions & 1,568 deletions src/Learning/KWDRRuleLibrary/KWDRTokenCounts.cpp

This file was deleted.

334 changes: 0 additions & 334 deletions src/Learning/KWDRRuleLibrary/KWDRTokenCounts.h

This file was deleted.

5 changes: 2 additions & 3 deletions src/Learning/KWData/KWCDUniqueString.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,8 @@ KWCDUniqueStringDataPtr KWCDUniqueStringDictionary::AsUniqueString(const ALStrin
pvUniqueStringDatas.SetAt(nHashPosition, pUniqueStringData);

// Retaillage dynamique
assert(GetHashTableSize() < INT_MAX / sizeof(void*));
if (GetCount() > GetHashTableSize() / 2)
ReinitHashTable(DictionaryGetNextTableSize(2 * GetHashTableSize()));
if (GetCount() > GetHashTableSize() / 2 and GetHashTableSize() < INT_MAX)
ReinitHashTable(DictionaryGetNextTableSize(2 * min(GetHashTableSize(), INT_MAX / 2)));
}
return pUniqueStringData;
}
Expand Down
7 changes: 3 additions & 4 deletions src/Learning/KWData/KWSymbol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ void KWSymbolDictionary::ReinitHashTable(int nNewHashSize)
// Affichage du debut de la methode
if (bDisplay)
{
cout << "ReinitHashTable (" << GetCount() << "," << GetHashTableSize() << ")";
cout << "Symbol ReinitHashTable (" << GetCount() << "," << GetHashTableSize() << ")";
cout << " -> " << nNewHashSize << ": " << flush;
timer.Start();
}
Expand Down Expand Up @@ -769,9 +769,8 @@ KWSymbolDataPtr KWSymbolDictionary::AsSymbol(const char* key, int nLength)
pvSymbolDatas.SetAt(nHashPosition, pSymbolData);

// Retaillage dynamique
assert(GetHashTableSize() < INT_MAX / sizeof(void*));
if (GetCount() > GetHashTableSize() / 2)
ReinitHashTable(DictionaryGetNextTableSize(2 * GetHashTableSize()));
if (GetCount() > GetHashTableSize() / 2 and GetHashTableSize() < INT_MAX)
ReinitHashTable(DictionaryGetNextTableSize(2 * min(GetHashTableSize(), INT_MAX / 2)));
}
return pSymbolData;
}
Expand Down
Loading

0 comments on commit 49aaec1

Please sign in to comment.