Discussion :

[Sennad]

Et bien si je fais :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
public void parseDirectory(String path, Connection conn) {
        ...
        PreparedStatement pstmt = null;
 
        try {
            for (File f : actual.listFiles()) {
                ...
            }
            pstmt.close();
        } catch (Exception e) {
            System.out.println("      Problem on " + path);
            Utils.ReportUtils.WriteInFile("Problem on " + path, Constants.LOGS_PATH);
        }
    }

Donc j'ouvre un seul PreparedStatement, et je le passe en paramètre de toutes les méthode réalisant des requêtes a la DB, je le ferme pas dans ces méthodes mais seulement a la fin de parseDirectory().
C'est pas bon ?

Et pour le try-with-rescources avec le resultSet je comprends pas, je peux pas faire ça :
try (ResultSet rs = pstmt.executeQuery()) {
pstmt = "query...";
}
Puisque la requête du pstmt est définie après la celle des ressources du try..

[adiGuba]

Donc j'ouvre un seul PreparedStatement, et je le passe en paramètre de toutes les méthode réalisant des requêtes a la DB, je le ferme pas dans ces méthodes mais seulement a la fin de parseDirectory().
C'est pas bon ?

Non ce n'est pas ce que tu fais : tu déclares une variable pstmt dans parseDirectoru() que tu initialises à null, et qui restera TOUJOURS à cette valeur.
Tu la passes bien aux autres méthodes mais cela ne sert à rien parce qu'elle est null. Du coup tu continues à en créer un nouveau à chaque fois dans chacune des méthodes.
Bref ca ne sert à rien...

Je rappelle quand même qu'on ne peut pas modifier un paramètre en le réaffectant dans une méthode...

Bref tu fais quelque chose du genre :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
 
	public void parseDirectory(String path, Connection conn) {
		// ...
		PreparedStatement pstmt = null; // Pourquoi ???
		try {
			for (File f : actual.listFiles()) {
				// ...
				updateCheckedColumn(actualPath, conn, pstmt); // pstmt est null !!!!
				// ...
			}
			// Pourquoi ????
			// Un close() en dehors d'un try/finally ou try-with-ressource
			// c'est jamais bon signe !!!
			pstmt.close();
		} catch (Exception e) {
			// ...
		}
	}
 
	public void updateCheckedColumn(String url, Connection conn,
			PreparedStatement pstmt) {
		try {
			// Tu crée un PreparedStatement que tu ne libères pas !!!
			pstmt = conn.prepareStatement("UPDATE ReferencesFiles SET Checked=1 WHERE URL=?");
			pstmt.setString(1, url);
			pstmt.executeUpdate();
		} catch (SQLException e) {
			System.out.println("Problem updating last checked date (" + url + ")");
		}
	}

Soit tu laisses comme c'était avant en créant le PreparedStatement dans chaque méthode, et dans ce cas il n'y a aucune raison de le créer dans parseDirectory() :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
	public void parseDirectory(String path, Connection conn) {
		// ...
		try {
			for (File f : actual.listFiles()) {
				// ...
				updateCheckedColumn(actualPath, conn);
				// ...
			}
		} catch (Exception e) {
			// ...
		}
	}
 
	public void updateCheckedColumn(String url, Connection conn) {
		// on ouvre une ressource dans un try-with-ressource
		// afin d'être sûr qu'elle soit libérée proprement :
		try (PreparedStatement pstmt = conn.prepareStatement("UPDATE ReferencesFiles SET Checked=1 WHERE URL=?")) {
			pstmt.setString(1, url);
			pstmt.executeUpdate();
		} catch (SQLException e) {
			System.out.println("Problem updating last checked date (" + url + ")");
		}
	}

Soit tu crées le PreparedStatement une seule fois dans parseDirectory() et tu le passes en paramètres de la méthode en question :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
	public void parseDirectory(String path, Connection conn) {
		// ...
		try {
			// on ouvre une ressource dans un try-with-ressource
			// afin d'être sûr qu'elle soit libérée proprement :
			try (PreparedStatement pstmt = conn.prepareStatement("UPDATE ReferencesFiles SET Checked=1 WHERE URL=?")) {
				for (File f : actual.listFiles()) {
					// ...
					updateCheckedColumn(actualPath, conn);
					// ...
				}
			}
		} catch (Exception e) {
			// ...
		}
	}
 
	// La méthode devrait être private, car il faut lui passer le bon prepared statement :
	private void updateCheckedColumn(String url, Connection conn, PreparedStatement pstmt) {
		try {
			pstmt.setString(1, url);
			pstmt.executeUpdate();
		} catch (SQLException e) {
			System.out.println("Problem updating last checked date (" + url
					+ ")");
		}
	}

De plus j'ai dit qu'avant de faire cela il fallait écrire une version itérative de parseDirectory(), sinon la récursion devient galère à gérer...

As-tu au moins fait des tests sans le System.gc() ? Car il y a des chances que cela soit plus gourmand que cela (et ca ne sert à rien d'optimiser un code sans tester ses performances avant).

Et pour le try-with-rescources avec le resultSet je comprends pas, je peux pas faire ça :
try (ResultSet rs = pstmt.executeQuery()) {
pstmt = "query...";
}
Puisque la requête du pstmt est définie après la celle des ressources du try..

Ben il suffit de les mettre dans le bon ordre.
C'est logique que tu ne puisses pas utiliser le pstmt si tu ne l'as pas initialisé avant... et cela n'a rien à voir avec le try-with-ressource...
Donc je ne vois toujours pas le problème...

a++

[Sennad]

Soit tu crées le PreparedStatement une seule fois dans parseDirectory() et tu le passes en paramètres de la méthode en question :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
public void parseDirectory(String path, Connection conn) {
		// ...
		try {
			// on ouvre une ressource dans un try-with-ressource
			// afin d'être sûr qu'elle soit libérée proprement :
			try (PreparedStatement pstmt = conn.prepareStatement("UPDATE ReferencesFiles SET Checked=1 WHERE URL=?")) {
				for (File f : actual.listFiles()) {
					// ...
					updateCheckedColumn(actualPath, conn);
					// ...
				}
			}
		} catch (Exception e) {
			// ...
		}
	}
 
	// La méthode devrait être private, car il faut lui passer le bon prepared statement :
	private void updateCheckedColumn(String url, Connection conn, PreparedStatement pstmt) {
		try {
			pstmt.setString(1, url);
			pstmt.executeUpdate();
		} catch (SQLException e) {
			System.out.println("Problem updating last checked date (" + url
					+ ")");
		}
	}

Mais j'ai pas les mêmes requêtes pour chaque méthode, du coup il faut que je créer un nouveau PreparedStatement a chaque fois que je vais appeler une méthode, donc ça sert a rien autant les créer dans la méthode non ? Je veux dire qu'on va en créer autant dans tout les cas

Mais tu a raison peut être que seulement le system.gc() suffira !

Et pour le try-with-rescources c'est pas grave je fais avec un finally et ça ira très bien..
( Si je les mets dans le bon ordre, je vais devoir refaire un try catch avant, donc ça revient au même.. )

Merci encore énormément !

[adiGuba]

Mais j'ai pas les mêmes requêtes pour chaque méthode, du coup il faut que je créer un nouveau PreparedStatement a chaque fois que je vais appeler une méthode, donc ça sert a rien autant les créer dans la méthode non ? Je veux dire qu'on va en créer autant dans tout les cas

Ben ce n'est pas pareil car tu appelles plusieurs fois les méthodes...
Après oui il faut dupliquer cela pour chaque méthode que tu appelles et cela alourdit le code, mais j'avais prévenu :

Attention toutefois car ces deux dernier point vont alourdir un peu le code, donc ce serait plutôt à faire en dernier moment si les perfs sont toujours mauvaise (à mon avis le System.gc() doit déjà faire bien mal).
De plus cela s'adapte assez mal avec le coté récursif du code, et dans ce cas il serait sûrement souhaitable d'envisager une version itérative...

Et pour le try-with-rescources c'est pas grave je fais avec un finally et ça ira très bien..
( Si je les mets dans le bon ordre, je vais devoir refaire un try catch avant, donc ça revient au même.. )

Franchement je ne comprend pas ton problème là dessus.
Si tu as un try/finally il suffit de le remplacer par le try-with-ressources...


a++

[Sennad]

Super, ça ira très très bien
Merci beaucoup pour le temps que tu a passé sur mon problème !
A bientôt !

[adiGuba]

Attention car si tu n'arrives pas à utiliser le try-with-ressources c'est que tu dois avoir une petite erreur dans ton code.

Une petite erreur c'est certe rien, mais répété un millions de fois ca peut faire très mal (surtout que tu ignores les exceptions).


a++

[tchize_]

Accessoirement, je ne sais pas où tu travaille, mais là où je suis, si je lance 5 millions de fois d'affilée md5sum, j'ai un sysadmin enragé sur le dos qui viens me demander pourquoi je sature sa machine depuis plusieurs heures

Créer un processus cela prends du temps. Je travaillerais plutot avec un outil du genre md5deep si tu ne veux pas coder toi même le checksum avec les apis java.

[Sennad]

Salut,

C'est l'équipe qui est root sur les machines qui m'a demandé de faire ce programme, on travaille ensemble depuis le début.
Les designer qui utilisent ces machines ne travaillent pas directement dessus, enfin c'est vachement compliqué.. 3000 employés sur le site, 45000 mondial, mon programme va tourner sur des sites à Singapour et à Catane (pour l'instant).
Enfin bref..

md5deep apporte de meilleures performances ?
Si j'ai bien compris ce que je trouve sur le net je vais devoir l'installer sur toutes les machines unix, et du coup faire un Runtime.getRuntime().exec("md5deep /mon/repertoire/...");
Ce que je faisais jusqu'à maintenant avec md5sum, mais en faisant MessageDigest.getInstance("md5"); j'ai gagné en performance, donc je suppose que je vais en reperdre en faisant Runtime.getRuntime().exec("md5deep /mon/repertoire/..."); sauf que si md5deep est plus performant que md5sum ça vaut peut-être le coup..

Merci !

[tchize_]

a tester. avec md5deep, tu passe plusieurs fichier pour un seul appel. Ca veux dire que tu économise plusieurs million de chargement d'exécutable, de préparation de contexte process, de mapping d'entrée sortie, de dynamic linking des librairies, bref tout le bordel que fait un OS à chaque démarrage d'un process. C'est en général négligeable, mais négligeable multiplié par 5 millions...

[Sennad]

Ok merci !
SI j'ai le temps avant de finir, j'essayerai et je comparerai

Merci encore !

[Sennad]

Salut à tous !

Je reviens ici car j'ai bien viré le system.gc() mais les perfs sont encore catastrophique :'(
J'ai choisis un répertoire de 4Mo.
Mon programme a mis presque 4min pour tout parcourir, hacher, insérer dans la db.
Si j'enlève toutes les étapes concernant la DB, c'est à dire que je parcours seulement le répertoire en faisant un hash que je stock nulle part ni rien, et là il met 2sec.....
Donc je pense qu'il y a un gros problème

J'ai donc repensé à ce qu'on avait dit avec le addBatch() et executeBatch(), mais je comprends pas trop comment le mettre en place..
Voilà mon code :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
package Dao.JDBC;
import ...
 
public class JdbcReferencesFilesDAO implements IReferencesFilesDAO {
 
    @Override
    public void parseDirectory(String path, Connection conn) {
        File actual = new File(path);
        String actualPath;
        String actualName;
        String hash;
        String oldHash;
 
        try {
            for (File f : actual.listFiles()) {
                actualPath = f.getAbsolutePath();
                actualName = f.getName();
                if (!f.getName().equals(".snapshot")) {
                    if (f.isDirectory()) {
                        parseDirectory(actualPath, conn);
                    } else {
                        Constants.counter++;
                        hash = Utils.HashUtils.hashFile(actualPath);
                        oldHash = getOldHashIfExist(actualPath, conn);
 
                        if (oldHash != null) {
                            if (oldHash.equals(hash)) {
                                updateCheckedColumn(actualPath, conn);
                            } else {
                                Utils.ReportUtils.WriteInFile("Reference File MODIFIED : " + actualPath, Constants.REFERENCES_REPORTING_PATH);
                                updateSignatureAndCheckedColumn(hash, actualPath, conn);
                            }
                        } else {
                            Utils.ReportUtils.WriteInFile("Reference File ADDED : " + actualPath, Constants.REFERENCES_REPORTING_PATH);
                            insertInReferencesFilesTableDB(actualName, actualPath, hash, conn);
                        }
                        System.out.print(".");
                        if (((Constants.counter) % 100) == 0) {
                            System.out.print("\n" + Constants.counter + "files found");
                        }
                    }
                }
            }
        } catch (Exception e) {
            System.out.println("      Problem on " + path);
            Utils.ReportUtils.WriteInFile("Problem on " + path, Constants.LOGS_PATH);
        }
    }
 
    @Override
    public void insertInReferencesFilesTableDB(String name, String url, String hash, Connection conn) {
 
        try (PreparedStatement pstmt = conn.prepareStatement("INSERT INTO ReferencesFiles (Name, URL, Signature, Checked) VALUES (?,?,?,?)")) {
            pstmt.setString(1, name);
            pstmt.setString(2, url);
            pstmt.setString(3, hash);
            pstmt.setInt(4, 1);
            pstmt.executeUpdate();
        } catch (SQLException e) {
            System.out.println("      Impossible to add " + name + " in ReferencesFiles Table !");
            Utils.ReportUtils.WriteInFile("Problem adding " + url + " file", Constants.LOGS_PATH);
        }
    }
 
    @Override
    public String getOldHashIfExist(String path, Connection conn) {
        ResultSet rs = null;
 
        try (PreparedStatement pstmt = conn.prepareStatement("SELECT Signature FROM ReferencesFiles WHERE URL=?")) {
            pstmt.setString(1, path);
            rs = pstmt.executeQuery();
            if (rs.first()) {
                String hash = rs.getString(1);
                return hash;
            } else {
                return null;
            }
        } catch (Exception e) {
            System.out.println("Problem checking if file of reference exist in DB (" + path + ")");
            Utils.ReportUtils.WriteInFile("Problem checking if file of reference exist in DB (" + path + ")", Constants.LOGS_PATH);
            return null;
        } finally {
            try {
                rs.close();
            } catch (SQLException ex) {
            }
        }
    }
 
    @Override
    public void updateCheckedColumn(String url, Connection conn) {
        try (PreparedStatement pstmt = conn.prepareStatement("UPDATE ReferencesFiles SET Checked=1 WHERE URL=?")) {
            pstmt.setString(1, url);
            pstmt.executeUpdate();
        } catch (SQLException e) {
            System.out.println("Problem updating last checked date (" + url + ")");
        }
    }
 
    @Override
    public void updateSignatureAndCheckedColumn(String url, String newHash, Connection conn) {
        try (PreparedStatement pstmt = conn.prepareStatement("UPDATE ReferencesFiles SET Signature=?, Checked=1 WHERE URL=?")) {
            pstmt.setString(1, newHash);
            pstmt.setString(2, url);
            pstmt.executeUpdate();
        } catch (SQLException e) {
            System.out.println("Problem updating signature (" + url + ")");
        }
    }
 
    @Override
    public void checkDeletedFiles(Connection conn) {
        ResultSet rs = null;
        try (PreparedStatement pstmt = conn.prepareStatement("SELECT URL FROM ReferencesFiles WHERE (Checked = 0)")) {
            rs = pstmt.executeQuery();
            while (rs.next()) {
                System.out.println("!!! Reference DELETED !!! -> " + rs.getString(1));
                deleteRecord(rs.getString(1), conn);
                Utils.ReportUtils.WriteInFile("Reference File DELETED " + rs.getString(1), Constants.REFERENCES_REPORTING_PATH);
            }
 
        } catch (Exception e) {
            e.printStackTrace();
            System.out.println("Problem checking deletedFiles");
            Utils.ReportUtils.WriteInFile("Problem checking deleted files", Constants.LOGS_PATH);
        } finally {
            try {
                rs.close();
            } catch (SQLException ex) {
            }
        }
    }
 
    @Override
    public void deleteRecord(String path, Connection conn) {
        try (PreparedStatement pstmt = conn.prepareStatement("DELETE FROM ReferencesFiles WHERE URL = ?")) {
            pstmt.setString(1, path);
            pstmt.executeUpdate();
        } catch (SQLException e) {
            System.out.println("Problem deleting " + path);
            Utils.ReportUtils.WriteInFile("Problem deleting " + path, Constants.LOGS_PATH);
        }
    }
}

Et si je fais seulement ca :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
public void parseDirectory(String path, Connection conn) {
        File actual = new File(path);
        String actualPath;
 
        try {
            for (File f : actual.listFiles()) {
                actualPath = f.getAbsolutePath();
                if (!f.getName().equals(".snapshot")) {
                    if (f.isDirectory()) {
                        parseDirectory(actualPath, conn);
                    } else {
                        hash = Utils.HashUtils.hashFile(actualPath);
                        System.out.print(".");
                        if (((Constants.counter) % 100) == 0) {
                            System.out.print("\n" + Constants.counter + "files found");
                        }
                    }
                }
            }
        } catch (Exception e) {
            System.out.println("      Problem on " + path);
            Utils.ReportUtils.WriteInFile("Problem on " + path, Constants.LOGS_PATH);
        }
    }

C'est 120 fois plus rapide ><
Si c’est normal je vais me pendre maintenant ^^
Sachant qu’à la fin mon programme doit parcourir 11To de donnée séparée sur 50 Processus différents et sur une 20aine de machines..

[adiGuba]

Avant de penser aux batch, il faut éviter la multiplication des PreparedStatement comme je l'avais indiqué.
Cela permet d'éviter que le driver JDBC parse à chaque fois la requête SQL...


Ensuite tu pourrais peut-être aussi revoir le traitement SQL.
Actuellement pour chaque hash tu fais un SELECT, puis un UPDATE ou un INSERT.
Si tu as bien une clef primaire, tu peux peut-être remplacer tout cela par un INSERT ON DUPLICATE KEY UPDATE (ou équivalent selon ta BDD)...


Enfin je ne comprend toujours pas pourquoi tu t'embêtes avec des try/finally pour fermer le ResultSet alors que tu es sous Java 7 et que tu utilises le try-with-ressources ailleurs :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
    @Override
    public String getOldHashIfExist(String path, Connection conn) {
        ResultSet rs = null;
 
        try (PreparedStatement pstmt = conn.prepareStatement("SELECT Signature FROM ReferencesFiles WHERE URL=?")) {
            pstmt.setString(1, path);
            rs = pstmt.executeQuery();
            if (rs.first()) {
                String hash = rs.getString(1);
                return hash;
            } else {
                return null;
            }
        } catch (Exception e) {
            System.out.println("Problem checking if file of reference exist in DB (" + path + ")");
            Utils.ReportUtils.WriteInFile("Problem checking if file of reference exist in DB (" + path + ")", Constants.LOGS_PATH);
            return null;
        } finally {
            try {
                rs.close();
            } catch (SQLException ex) {
            }
        }
    }

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
    @Override
    public String getOldHashIfExist(String path, Connection conn) {
        try (PreparedStatement pstmt = conn.prepareStatement("SELECT Signature FROM ReferencesFiles WHERE URL=?")) {
            pstmt.setString(1, path);
            try (ResultSet rs = pstmt.executeQuery()) {
                if (rs.first()) {
                    String hash = rs.getString(1);
                    return hash;
                } else {
                    return null;
                }
            }
        } catch (Exception e) {
            System.out.println("Problem checking if file of reference exist in DB (" + path + ")");
            Utils.ReportUtils.WriteInFile("Problem checking if file of reference exist in DB (" + path + ")", Constants.LOGS_PATH);
            return null;
        }
    }

a++

[tchize_]

Il y a combien de fichiers dans les 4MO que tu as scanné? On peux voire le code de

getOldHashIfExist(actualPath, conn);
updateCheckedColumn(actualPath, conn);
updateSignatureAndCheckedColumn(hash, actualPath, conn);
insertInReferencesFilesTableDB(actualName, actualPath, hash, conn);

Ainsi que la DDL des tables concernées.

Je vois déjà un problème majeur: pour chaque fichier, a priori, tu fais deux requêtes SQL, et pas toujours les même, alors que tu pourrais laisser la BD faire son boulot. Tu récupère un hash (donc un SELECT de un élément parmis un million?) Tu n'a pas besoin, par exemple, de savoir si la ligne existe.

En l'occurence, tu peux très bien faire sous mysql

Code sql :

Sélectionner tout - Visualiser dans une fenêtre à part

insert into Tatable(name,path,hash) values ('lenom','lepath','lehash') ON DUPLICATE KEY UPDATE

combiné à des insertions multiples en JDBc, tu peux faire:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
public void updateHashes(List<Item> items, Connection connec){
    String sql = "insert into Tatable(name,path,hash) values (?,?,?) ON DUPLICATE KEY UPDATE";
    try (preparedStatement =  connection.prepareStatement(sql)){
      items.stream().forEach(it -> {
          preparedStatement.setString(1, it.getName());
          preparedStatement.setString(2, it.getFullPath());
          preparedStatement.setString(3, it.getHash());
          preparedStatement.addBatch();
 
          }
      );    
      int[] affectedRecords = preparedStatement.executeBatch();
    // + nettoyage etc pas mis ici
    } catch (.....) {.....}
}

[Sennad]

Salut,

D'abord merci pour ta réponse !

Avant de penser aux batch, il faut éviter la multiplication des PreparedStatement comme je l'avais indiqué. Cela permet d'éviter que le driver JDBC parse à chaque fois la requête SQL...

Je comprends pas, je peux pas en faire moins que ça, c'est pareil que je les mette dans la méthode ou dans parseDirectory() il faudra en faire autant dans tous les cas.. Je comprends pas la différence..

Ensuite tu pourrais peut-être aussi revoir le traitement SQL.
Actuellement pour chaque hash tu fais un SELECT, puis un UPDATE ou un INSERT.
Si tu as bien une clef primaire, tu peux peut-être remplacer tout cela par un INSERT ON DUPLICATE KEY UPDATE (ou équivalent selon ta BDD)...

Okey c'est noté je ne connaissais pas !

Enfin je ne comprend toujours pas pourquoi tu t'embêtes avec des try/finally pour fermer le ResultSet alors que tu es sous Java 7 et que tu utilises le try-with-ressources ailleurs :

J'avais entendu dire qu'un try catch pompais beaucoup de mémoire, et que du coup il fallait le plus possible éviter d'en faire.. Mais bon si tu me dis que c'est mieux je vais le faire

Merci encore, mais tu penses que je vais gagner significativement en perf avec ça ?

[Sennad]

Salut !

Il y a combien de fichiers dans les 4MO que tu as scanné?

135..

On peux voire le code de

getOldHashIfExist(actualPath, conn);
updateCheckedColumn(actualPath, conn);
updateSignatureAndCheckedColumn(hash, actualPath, conn);
insertInReferencesFilesTableDB(actualName, actualPath, hash, conn);

Elles sont dans post au quel tu viens de répondre

Ainsi que la DDL des tables concernées.

Là ca va être plus compliqué mais je vous met en PJ le schéma de la DB.

Tu récupère un hash (donc un SELECT de un élément parmi un million?) Tu n'a pas besoin, par exemple, de savoir si la ligne existe.

SI pasque si le hash n’existe pas déjà, ça veut dire que le fichier est nouveau (Je rappelle que mon programme tourne toutes les semaines), si le hash existe déjà mais que ce n’est pas le même, c'est que le fichier a été modifier.

combiné à des insertions multiples en JDBc, tu peux faire:

Ça c'est cool, j'vais essayer

Merci !

[adiGuba]

On peux voire le code de

getOldHashIfExist(actualPath, conn);
updateCheckedColumn(actualPath, conn);
updateSignatureAndCheckedColumn(hash, actualPath, conn);
insertInReferencesFilesTableDB(actualName, actualPath, hash, conn);

Le code est présent, il faut juste scrollé dans le bloc code pour le voir

Cela correspond à chaque fois à une requête SQL (new PreparedStatement + execute).



Sinon :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

      int[] affectedRecords = preparedStatement.executeBatch();

Attention quand même à executeBatch(), car si c'est plus performant c'est aussi plus couteux en mémoire.
Cela peut être dangereux de ne faire qu'un seul executeBatch() à la fin.
Il vaut mieux regrouper les batch par "petit" bout (style toutes les 100, 500 ou 1000 requêtes). Enfin à tester mais je pense qu'il vaut mieux éviter de faire des batch d'1 millions de requêtes par exemple.

Je comprends pas, je peux pas en faire moins que ça, c'est pareil que je les mette dans la méthode ou dans parseDirectory() il faudra en faire autant dans tous les cas.. Je comprends pas la différence..

Actuellement tu vas créer 2 PreparedStatement pour chacun des fichiers que tu analyses (un pour le SELECT, l'autre pour UPDATE ou INSERT).
Si tu traites 1000 fichiers tu vas créer 2000 PreparedStatement, et le drivers JDBC va donc devoir analyser 2000 requêtes SQL...
Bref grosso-modo ton code fait ceci :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
for (...) {
   try (PreparedStatement stmt = con.prepareStatement("...")) {
      ...
      stmt.executeUpdate();
   }
}

Je te dis juste de remonter la création des PreparedStatement pour les sortir de la boucle, comme ceci :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
try (PreparedStatement stmt = con.prepareStatement("...")) {
    for (...) {
         stmt.clearParameters();
         ...
         stmt.executeUpdate();
   }
}

Du coup le driver JDBC ne va analyser qu'une seule fois la requête SQL (quelque soit le nombre d'élément que tu traites), mais l'exécuter plusieurs fois...




De toute manière si tu veux utiliser les batch tu dois d'abord faire cela (sinon tu ne pourras faire que des batch d'1 requête ce qui est inutile).

J'avais entendu dire qu'un try catch pompais beaucoup de mémoire, et que du coup il fallait le plus possible éviter d'en faire..

C'est complètement faux.
Ce qui peut être couteux c'est la génération d'une exception (le stacktrace), mais cela n'a rien à voir avec le présence (ou non) d'un try/catch ou autre...

Mais bon si tu me dis que c'est mieux je vais le faire

Libérer les ressources c'est toujours important. Et lorsqu'on doit traiter des millions d'éléments c'est même primordiale.



a++

[Sennad]

Merci pour vos réponses.

Sinon :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

int[] affectedRecords = preparedStatement.executeBatch();

Attention quand même à executeBatch(), car si c'est plus performant c'est aussi plus couteux en mémoire.
Cela peut être dangereux de ne faire qu'un seul executeBatch() à la fin.
Il vaut mieux regrouper les batch par "petit" bout (style toutes les 100, 500 ou 1000 requêtes). Enfin à tester mais je pense qu'il vaut mieux éviter de faire des batch d'1 millions de requêtes par exemple.

Ok mais du coup je fais dans mes méthodes qui insèrent dans la db pstmt.addBatch(), et une fois qu'il a fait 100 fichier par exemple j’appelle une méthode qui prend en paramètre ce pstmt et qui fait executeBatch() ?
C'est pas très clair ce que je viens de dire, je vais faire un truc, vous le mettre et tu me dira si c'est bon ?

Je te dis juste de remonter la création des PreparedStatement pour les sortir de la boucle, comme ceci :

Ok mais j'ai plusieurs requêtes complétement différente, du coup je peux créer plusieurs preparedStatement avant la grande boucle et les mettre en paramètre de la méthode qui correspond ?

J'avais entendu dire qu'un try catch pompais beaucoup de mémoire, et que du coup il fallait le plus possible éviter d'en faire..

C'est complètement faux.
Ce qui peut être couteux c'est la génération d'une exception (le stacktrace), mais cela n'a rien à voir avec le présence (ou non) d'un try/catch ou autre...

Ok parfait

[Sennad]

Voilà ce que j'ai fais, j'espere pas avoir fais de bêtises !

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
package Dao.JDBC;
 
import Constants.Constants;
import Dao.IReferencesFilesDAO;
import java.io.File;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.logging.Level;
import java.util.logging.Logger;
 
public class JdbcReferencesFilesDAO implements IReferencesFilesDAO {
 
    @Override
    public void parseDirectory(String path, Connection conn) {
        File actual = new File(path);
        String actualPath;
        String actualName;
        String hash;
        String oldHash;
        Constants.counter = 0;
        try {
            PreparedStatement pstmtInsertInReferencesFilesTableDB = conn.prepareStatement("INSERT INTO ReferencesFiles (Name, URL, Signature, Checked) VALUES (?,?,?,?)");
            PreparedStatement pstmtGetOldHashIfExist = conn.prepareStatement("SELECT Signature FROM ReferencesFiles WHERE URL=?");
            PreparedStatement pstmtUpdateCheckedColumn = conn.prepareStatement("UPDATE ReferencesFiles SET Checked=1 WHERE URL=?");
            PreparedStatement pstmtUpdateSignatureAndCheckedColumn = conn.prepareStatement("UPDATE ReferencesFiles SET Signature=?, Checked=1 WHERE URL=?");
            try {
                for (File f : actual.listFiles()) {
                    actualPath = f.getAbsolutePath();
                    actualName = f.getName();
                    if (!f.getName().equals(".snapshot")) {
                        if (f.isDirectory()) {
                            parseDirectory(actualPath, conn);
                        } else {
                            Constants.counter++;
                            hash = Utils.HashUtils.hashFile(actualPath);
                            oldHash = getOldHashIfExist(actualPath, conn, pstmtGetOldHashIfExist);
 
                            if (oldHash != null) {
                                if (oldHash.equals(hash)) {
                                    updateCheckedColumn(actualPath, conn, pstmtUpdateCheckedColumn);
                                } else {
                                    Utils.ReportUtils.WriteInFile("Reference File MODIFIED : " + actualPath, Constants.REFERENCES_REPORTING_PATH);
                                    updateSignatureAndCheckedColumn(hash, actualPath, conn, pstmtUpdateSignatureAndCheckedColumn);
                                }
                            } else {
                                Utils.ReportUtils.WriteInFile("Reference File ADDED : " + actualPath, Constants.REFERENCES_REPORTING_PATH);
                                insertInReferencesFilesTableDB(actualName, actualPath, hash, conn, pstmtInsertInReferencesFilesTableDB);
                            }
                            System.out.print(".");
                            if (((Constants.counter) % 100) == 0) {
                                executeBatch(pstmtGetOldHashIfExist);
                                executeBatch(pstmtInsertInReferencesFilesTableDB);
                                executeBatch(pstmtUpdateCheckedColumn);
                                executeBatch(pstmtUpdateSignatureAndCheckedColumn);
                                System.out.print("\n" + Constants.counter + "files found");
                            }
                        }
                    }
                }
                executeBatch(pstmtGetOldHashIfExist);
                executeBatch(pstmtInsertInReferencesFilesTableDB);
                executeBatch(pstmtUpdateCheckedColumn);
                executeBatch(pstmtUpdateSignatureAndCheckedColumn);
                System.out.print("\n" + Constants.counter + "files found");
            } catch (Exception e) {
                System.out.println("      Problem on " + path);
                Utils.ReportUtils.WriteInFile("Problem on " + path, Constants.LOGS_PATH);
            } finally {
                try {
                    pstmtInsertInReferencesFilesTableDB.close();
                    pstmtGetOldHashIfExist.close();
                    pstmtUpdateCheckedColumn.close();
                    pstmtUpdateSignatureAndCheckedColumn.close();
                } catch (SQLException ex) {
                    System.out.println("ERROR");
                }
            }
        } catch (Exception e) {
            throw new RuntimeException();
        }
    }
 
    @Override
    public void insertInReferencesFilesTableDB(String name, String url, String hash, Connection conn, PreparedStatement pstmt) {
        try {
            pstmt.clearParameters();
            pstmt.setString(1, name);
            pstmt.setString(2, url);
            pstmt.setString(3, hash);
            pstmt.setInt(4, 1);
            pstmt.addBatch();
        } catch (SQLException e) {
            System.out.println("      Impossible to add " + name + " in ReferencesFiles Table !");
            Utils.ReportUtils.WriteInFile("Problem adding " + url + " file", Constants.LOGS_PATH);
        }
    }
 
    @Override
    public String getOldHashIfExist(String path, Connection conn, PreparedStatement pstmt) {
        ResultSet rs = null;
 
        try {
            pstmt.clearParameters();
            pstmt.setString(1, path);
            rs = pstmt.executeQuery();
            if (rs.first()) {
                String hash = rs.getString(1);
                return hash;
            } else {
                return null;
            }
        } catch (Exception e) {
            System.out.println("Problem checking if file of reference exist in DB (" + path + ")");
            Utils.ReportUtils.WriteInFile("Problem checking if file of reference exist in DB (" + path + ")", Constants.LOGS_PATH);
            return null;
        } finally {
            try {
                rs.close();
            } catch (SQLException ex) {
            }
        }
    }
 
    @Override
    public void updateCheckedColumn(String url, Connection conn, PreparedStatement pstmt) {
        try {
            pstmt.clearParameters();
            pstmt.setString(1, url);
            pstmt.addBatch();
        } catch (SQLException e) {
            System.out.println("Problem updating last checked date (" + url + ")");
        }
    }
 
    @Override
    public void updateSignatureAndCheckedColumn(String url, String newHash, Connection conn, PreparedStatement pstmt) {
        try {
            pstmt.clearParameters();
            pstmt.setString(1, newHash);
            pstmt.setString(2, url);
            pstmt.addBatch();
        } catch (SQLException e) {
            System.out.println("Problem updating signature (" + url + ")");
        }
    }
 
    @Override
    public void checkDeletedFiles(Connection conn, PreparedStatement pstmt) {
        ResultSet rs = null;
        try {
            pstmt.clearParameters();
            rs = pstmt.executeQuery();
            while (rs.next()) {
                System.out.println("!!! Reference DELETED !!! -> " + rs.getString(1));
                deleteRecord(rs.getString(1), conn);
                Utils.ReportUtils.WriteInFile("Reference File DELETED " + rs.getString(1), Constants.REFERENCES_REPORTING_PATH);
            }
 
        } catch (Exception e) {
            e.printStackTrace();
            System.out.println("Problem checking deletedFiles");
            Utils.ReportUtils.WriteInFile("Problem checking deleted files", Constants.LOGS_PATH);
        } finally {
            try {
                rs.close();
            } catch (SQLException ex) {
            }
        }
    }
 
    @Override
    public void deleteRecord(String path, Connection conn) {
        try (PreparedStatement pstmt = conn.prepareStatement("DELETE FROM ReferencesFiles WHERE URL = ?")) {
            pstmt.setString(1, path);
            pstmt.executeUpdate();
        } catch (SQLException e) {
            System.out.println("Problem deleting " + path);
            Utils.ReportUtils.WriteInFile("Problem deleting " + path, Constants.LOGS_PATH);
        }
    }
 
    public void executeBatch (PreparedStatement pstmt) {
        try {
            pstmt.executeBatch();
        } catch (SQLException ex) {
            System.out.println("Impossible to execute batch");
        }
    }
 
}

Oui il reste aussi les resultSet a mettre en try-with-rescources !

Merci encore

[Sennad]

J'ai également remarqué qu'avec le autoCommit true c'était 2 fois plus lent (Sur une boucle de 0 à 10000) que si je commit à la fin, donc je vais mettre autoCommit false, et je commit en mémé temps que j'appelle la méthode qui fait executeBatch (tous les 100 fichiers pour l'instant).
Je vais lancer quelques tests.

[tchize_]

Là ca va être plus compliqué mais je vous met en PJ le schéma de la DB.

C'est con, il manque la partie dont j'(avais besoin, les ddl de l'unique table que tu utilise, pour voir si il n'y a pas de bourdes performances dedans

SI pasque si le hash n’existe pas déjà, ça veut dire que le fichier est nouveau (Je rappelle que mon programme tourne toutes les semaines), si le hash existe déjà mais que ce n’est pas le même, c'est que le fichier a été modifier.

Sauf que, à part logger, tu ne fais rien de ce traitement différencié. Si c'est pour avoir le même résultat dans tous les cas, c'est inutile de différencier

Si tu as absolument besoin de connaitre les Hash à l'avance pour savoir si c'est nouveau / modifié, récupère avec une seule requête tous les hash de ton répertoire, plutot que de le faire fichier par fichier. Il est bien plus rapide de récupérer 200 lignes en une seule fois que de demander 50 fois une seule ligne à une DB. Pour rappel, les DB sont optimisée pour traiter des données en masse, en général, plus que pour faire des aller-retour réseau