Discussion :

[kahya]

1. En limitant leurs durées de vie, ce que je t'ai dit dans mon premier post, c'est à dire en évitant de les déclarer en tant qu'attribut d'instance de la classe, mais uniquement dans le bloc où elles sont utilisées ( où sont déclaré CommandeEnCourLIST, maCommandeEnCourLIST, ...)

Je l'ai fais au maximum mais j'ai besoin d'en garder quelques unes quand meme.

2. En évitant de faire des copies des List/Map si cela n'est pas vraiment utile. La mémoire sera bien libéré dans tous les cas mais cela évite de faire des allocations/libérations de mémoire inutile...

La copie c'est parce que dans mes traitements je delete les lignes dans une des List/Map mais j'ai besoin d'en garder un exemplaire complet pour la suite.

J'ai rajouté des = null (exemple maCommandeEnCourLIST = null; ) pour essayer de liberer les ressources plus vite mais ça n'a pas l'air de marcher... je crois que mon prog est destiné à être lent

[adiGuba]

Je l'ai fais au maximum mais j'ai besoin d'en garder quelques unes quand meme.

Pourquoi ? Cela ressemble à une erreur de conception à mon avis

La copie c'est parce que dans mes traitements je delete les lignes dans une des List/Map mais j'ai besoin d'en garder un exemplaire complet pour la suite.

Je ne vois pas ces suppressions dans ton code... je suppose qu'elle sont effectué dans les autres méthodes que tu utilises (re-)

je crois que mon prog est destiné à être lent

Je crois surtout qu'il faudrait repensé un peu tout ca... car si c'est du même acabit que pour la lecture du fichier que tu faisais plusieurs milliers de fois c'est sûr que cela doit être lent...

a++

[kahya]

Pourquoi ? Cela ressemble à une erreur de conception à mon avis

Parce que je m'en sers dans plusieurs methodes, par exemple articlesMAP je le remplis à chaque fois que je construis un article (donc dans la methode constuireArticle() ), et ensuite j'en ai besoin dans construireCommande(). Je pourrais faire un return mais ça veut dire qu'à chaque fois que je construis un article il faudrait que je range le resultat de construireArticle() dans une variable alors que j'en ai pas besoin...

Je ne vois pas ces suppressions dans ton code... je suppose qu'elle sont effectué dans les autres méthodes que tu utilises (re-)

Voui, je peux te mettre le code des methodes aussi mais bon ça va commencer à faire beaucoup de code et ça doit plus etre tres clair..

[adiGuba]

Parce que je m'en sers dans plusieurs methodes, par exemple articlesMAP je le remplis à chaque fois que je construis un article (donc dans la methode constuireArticle() ), et ensuite j'en ai besoin dans construireCommande(). Je pourrais faire un return mais ça veut dire qu'à chaque fois que je construis un article il faudrait que je range le resultat de construireArticle() dans une variable alors que j'en ai pas besoin...

Désolé mais je trouve que c'est une solution de facilité pas très propre...
C'est d'autant plus difficile à comprendre ce que fait chaque méthode puisqu'elles semblent partager un grand nombre d'attributs en tant que variables locales...

Si dans quelques mois tu te replonges dans l'application tu risque d'avoir bien du mal à t'y retrouver...

a++

[kahya]

OK je n'ai plus de HashMap en variable de classe maintenant, je les ai faites passer de methode en methode, je ne sais pas si ça améliore grand chose, puisque je les ai mises dans ma methode lireFichier(), ça descend juste d'un cran...

[adiGuba]

OK je n'ai plus de HashMap en variable de classe maintenant, je les ai faites passer de methode en methode, je ne sais pas si ça améliore grand chose, puisque je les ai mises dans ma methode lireFichier(), ça descend juste d'un cran...

Et bien il ne te reste plus qu'a revoir tes méthodes une à une pour trouver l'origine du problème... et je te garantie qu'en limitant la porté des variables ce sera bien plus facile à faire

a++

[kahya]

Je pense que ces methodes sont correcte, il n'y a donc que le HashMap final contenant toutes les commandes, et que je serialise à la fin qui grossit (et une autre contenant un autre objet assez basique). Mis à part ça rien ne monopolise de ressources donc je suppose que je ne peux pas optimiser plus la durée de l'execution... Tanpis ils devront être patients lol merci quand même =)

[adiGuba]

Je pense que ces methodes sont correcte, (...). Mis à part ça rien ne monopolise de ressources donc je suppose que je ne peux pas optimiser plus la durée de l'execution...

Franchement j'en doute : car 24 heures pour analyser un fichier de 5 Mo je trouve ca vraiment énorme !!!

a++

[kahya]

Bein moi aussi...
http://files-upload.com/files/546837/ImportCommande.txt

j'ai mis mon code en entier ici si tu veux y jetter un oeil, tu en a déjà vu une bonne partie. Les methodes que tu n'a pas vu en général c'est juste des affectations de variables et constructions d'objet, avec quelques acces fichiers. Les acces fichiers prennent un peu plus de temps ça je veux bien, mais pas "de plus en plus de temps", c'est ça que je ne comprends pas, l'appli n'est pas lente tout le temps, elle est de plus en plus lente, et c'est ça que je voudrai resoudre parce qu'elle arrive vite à être très lente :/

[kahya]

Ah bon bein j'ai trouvé! C'etait qu'en fait à chaque création d'un lot j'accedais à un fichier, j'en recuperais une hashmap, j'y ajoutais mon lot, et je le réécrivais, et du coup la HashMap à réécrire était de plus en plus grosse forcément, donc l'operation prennait de plus en plus de temps Bon bein merci à tous et surtout adiGuba ^^

[kahya]

Par contre ça marche pour mon fichier de 8000 lignes mais pas pour celui de 54000, quelqu'un a une idée?

[adiGuba]

J'ai juste regarder le code de isolerArticle() :

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 ArrayList isolerArticle (ArrayList maCommandeEnCourLIST)
	{
		ArrayList monArticleLIST = new ArrayList();
		HashMap maLigneCommande = new HashMap();
		String maRefArticle;
 
		monArticleLIST.add(maCommandeEnCourLIST.get(0));
		maCommandeEnCourLIST.remove(0);
		maLigneCommande = (HashMap)monArticleLIST.get(0);
		maRefArticle = maLigneCommande.get("D").toString();
 
 
		for (int i = 0; i<maCommandeEnCourLIST.size(); i++)
		{
			maLigneCommande = (HashMap) maCommandeEnCourLIST.get(i);
 
			if (maLigneCommande.get("H").toString().compareTo(maRefArticle) == 0)
			{
				monArticleLIST.add(maLigneCommande);
				maCommandeEnCourLIST.remove(i);
			}
		}
		return (monArticleLIST);
	}

Déjà lors de l'appel, tu recrée une ArrayList à partir de l'ArrayList créer dans isolerArticle() : bref tu utilises un doublons pour rien :

Code :

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

articleEnCourLIST = new ArrayList(isolerArticle (tempCommandeEnCourLIST));

la liste retourné par isolerArticle() sera perdu et donc détruite par le Garbage Collector... donc la copie est inutile (autant utiliser directement ce qui t'es renvoyé :

Code :

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

articleEnCourLIST = isolerArticle (tempCommandeEnCourLIST);

Et plus généralement tu as énormément de création d'objet inutile :

Code :

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

1
2
3
4
5
	HashMap maLigneCommande = new HashMap();
 
	// Code qui n'utilise pas 'maLigneCommande'
 
	maLigneCommande = (HashMap)monArticleLIST.get(0);

Le HashMap que tu crées au tout début ne sert à rien car il n'est jamais utilisé ! Tu peux donc supprimer le new !
Cela peut sembler être rien du tout, mais sur 54 000 itérations cela représente 54 000 allocations/suppressions d'objets inutiles...


De même, a quoi te sert les codes du genre :

Code :

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

1
2
	articlesMAP = null;
	articlesMAP = new HashMap();

Surtout lorsque c'est fait en fin de méthode sur des variables locales...




Autres remarques moins "importante" pour les perfs, mais plus pour le coté évolutif du code : essaye de t'astraire de l'implémentation en utilisant les interfaces List/Map plutôt que les types concrets ArraList/HashMap autant que possible... tu comprendra le jour où tu devras utiliser une autre implémentation...





Tu peux aussi limiter le nombre d'accès via index, par exemple ici tu as 3 accès par index :

Code :

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

1
2
3
	monArticleLIST.add(maCommandeEnCourLIST.get(0));
	maCommandeEnCourLIST.remove(0);
	maLigneCommande = (HashMap)monArticleLIST.get(0);

alors qu'un seul suffirait (même si je ne pense pas que cela impacte vraiment les performances pour une ArrayList... mais sur une HashMap cela pourrait apporter quelque chose)

Code :

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

1
2
	maLigneCommande = (Map) maCommandeEnCourLIST.remove(0);
	monArticleLIST.add(maLigneCommande);

Pire : il y a même un bug dans la méthode au niveau du for : si tu supprimes un éléments dans la liste tu en saute un pendant le traitement.
Exemple :

1. Tu es sur la ligne 2 et tu rentres dans ta condition.
2. Tu supprimes la ligne 2, donc toutes les lignes sont décalés (la ligne 3 devient ligne 2)
3. Tu reviens au début du for qui incrémente i, et traite donc la ligne 3 (anciennement line 4), et donc l'ancienne ligne 3 est "sauté".

Bref déjà ton isolerArticle() pourrait ressembler à ceci :

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 List isolerArticle (List maCommandeEnCourLIST)
	{
		ArrayList monArticleLIST = new ArrayList();
		Map maLigneCommande = (Map)maCommandeEnCourLIST.remove(0);;
		String maRefArticle = maLigneCommande.get("D").toString();
 
		monArticleLIST.add(maLigneCommande);
 
		Iterator iter = maCommandeEnCourLIST.iterator();
		while (iter.hasNext()) {
			maLigneCommande = (Map) iter.next();
 
			if (maLigneCommande.get("H").toString().compareTo(maRefArticle) == 0) {
				monArticleLIST.add(maLigneCommande);
				iter.remove();
			}
 
		}
 
		// On redimentionne l'ArrayList à la taille utile
		monArticleLIST.trimToSize();
		return (monArticleLIST);
	}

Pour le reste désolé mais je n'ai pas vraiment envie de tout relire, surtout que cela regorde de list et de map temporaire et que ce n'est pas très facile à lire...


Bon courage...

a++

[PP(Team)]

Lol, pour quelqu'un qui n'a pas le temps, je trouve que tu l'as beaucoup aidé.

En bref, regarde comment tu pourrais optimiser ton code. Ce n'est pas facile, mais c'est possible.

[kahya]

Eeehhh bein! Vivement les cours lol! J'ai appris à la va-vite donc mon code n'est vraiment pas optimisé!

En tout cas merci enormément d'avoir pris le temps de lire tout ça ^^
J'ai corrigé mon code avec ce que tu m'a dis =)

Et sinon pour mes 54 000 lignes, quelqu'un a une idée?

[Napalm51]

essayes d'appliquer la même méthode de correction de tes fonctions que ce qu'adiGuba à pu faire mais sur toutes les autres méthodes ... ça va être long mais ça devrait corriger le problème, cherches un peu aussi

[kahya]

-_- J'ai di que simplement en copiant les lignes dans une hashmap (donc sans utiliser aucunes de mes methodes) ça plante quand meme donc ça ne vient pas de mes methodes....

[adiGuba]

-_- J'ai di que simplement en copiant les lignes dans une hashmap (donc sans utiliser aucunes de mes methodes) ça plante quand meme donc ça ne vient pas de mes methodes....

De plus je trouve que tu abuses vraiment des structures de données ! Ne serait-il pas mieux d'utiliser un objet métier comme valeur de retour de la méthode decouperLigne() ???

Les HashMap demande un certain espace mémoire pour permettre un accès rapide aux données. Dans ton cas un objet métier serait bien plus adapté (surtout que je ne pense pas que le format du fichier soit variable).

a++

[kahya]

Justement si, enfin j'ai plusieurs fichiers d'import, qui servent à créer un ou plusieurs objets, et ces fichiers n'ont pas la meme structure. De plus la methode doit fonctionner pour différents clients (donc qui ne génèreront pas forcément les memes fichiers avec la meme structures puisqu'ils n'utiliseront pas les memes logiciels). Donc la structure d'une classe metier devrait être modifiée à chaque fois, c'est pour ça que je fais comme ça. J'ai trouvé comment modifier la taille de la mémoire allouée à la JVM et maintenant tout roule ^^ merci encore.

[Napalm51]

Juste pour information, ton programme met combien de temps pour parser les 54k lignes ?

[bassim]

De plus la methode doit fonctionner pour différents clients (donc qui ne génèreront pas forcément les memes fichiers avec la meme structures puisqu'ils n'utiliseront pas les memes logiciels)

Je pense qu'il y a moyen de factoriser tout ça par une classe plus générale
peut être un problème de conception !