|
Hi Jon,
adding some GC.Collect() calls is not the way to fix programming mistakes. What you need to do is call Dispose() on those objects you don't need any longer, provided their class has such a method.
In your case, insertLib = new OleDbCommand(insSql, musLibConn); is the most likely culprit: inside the loop you create OleDbCommand instances, use them, then let them fade into oblivia (by assigning a new OleDbCommand to insertLib, the old one is gone, but still occupies memory.
I suggest you do not declare OleDbCommand insertLib; outside the foreach loop, instead do
foreach (...) {
OleDbCommand insertLib=new OleDbCommand(...);
...
insertLib.Dispose();
}
that makes it perfectly clear the OleDbCommand is always created, used, disposed.
I am not familiar with some of the classes you are using (e.g. ID3Info), chances are they also have a Dispose() and need the same treatment.
ADDED
There is a useful construct that automates the above, like so:
foreach (...) {
using (OleDbCommand insertLib=new OleDbCommand(...)) {
...
}
}
The advantages are it takes less code, and the Dispose is guaranteed to occur, even when an exception gets thrown inside the using block.
/ADDED
Luc Pattyn [Forum Guidelines] [My Articles]
- before you ask a question here, search CodeProject, then Google
- the quality and detail of your question reflects on the effectiveness of the help you are likely to get
- use the code block button (PRE tags) to preserve formatting when showing multi-line code snippets
modified on Wednesday, March 11, 2009 8:43 PM
|
|
|
|
|
I assume that the ID3info class (not shown) opens the file from the supplied parameters.
CurrSong = new ID3Info(filepaths[i], true);
If you have access to this classes code, make sure that the stream is disposed, if not, check to see if it has a Close or Dispose method. That might mean moving the
ID3Info CurrSong; inside the foreach loop.
DaveBTW, in software, hope and pray is not a viable strategy. (Luc Pattyn)Visual Basic is not used by normal people so we're not covering it here. (Uncyclopedia)
|
|
|
|
|
On a minor note... you have
foreach(string str in filepaths) then
path = filepaths[i].ToString(); The path field is unecessary, you can just use str in it's place.
DaveBTW, in software, hope and pray is not a viable strategy. (Luc Pattyn)Visual Basic is not used by normal people so we're not covering it here. (Uncyclopedia)
|
|
|
|
|
I have brought the declarations inside the loop, removed the System.GC.Collect and put in the insertLib.dispose(). The ID3Info class has no dispose or close method. Running the code now doesnt get me as far as with System.GC.Collect.
|
|
|
|
|
Check out Luc's added note to his previous post about wraping in a using block.
That might be the answer to the ID3Info as well (if that's the culprit). You can have nested using blocks
DaveBTW, in software, hope and pray is not a viable strategy. (Luc Pattyn)Visual Basic is not used by normal people so we're not covering it here. (Uncyclopedia)
|
|
|
|
|
Works for the OleDbCommand but not the ID3Info
It give s me this
'ID3.ID3Info': type used in a using statement must be implicitly convertible to 'System.IDisposable'
|
|
|
|
|
I've had a quick look at that code, each time you call ID3Info(string FilePath, bool LoadData), it calls ID3v1(string FilePath, bool LoadData) which calls Load() in the same class (if LoadData is true) which has FS.Close(); as the penultimate line, so that shouldn't be the source of your problem.
You could try changing the ID3v1 Load method to:
using (FileStreamEx FS = new FileStreamEx(_FilePath, FileMode.Open))
{
_HaveTag = false;
if (FS.HaveID3v1())
{
_Title = FS.ReadText(30, TextEncodings.Ascii);
FS.Seek(-95, SeekOrigin.End);
_Artist = FS.ReadText(30, TextEncodings.Ascii);
FS.Seek(-65, SeekOrigin.End);
_Album = FS.ReadText(30, TextEncodings.Ascii);
FS.Seek(-35, SeekOrigin.End);
_Year = FS.ReadText(4, TextEncodings.Ascii);
FS.Seek(-31, SeekOrigin.End);
_Comment = FS.ReadText(28, TextEncodings.Ascii);
FS.Seek(-2, SeekOrigin.End);
_TrackNumber = FS.ReadByte();
_Genre = FS.ReadByte();
_HaveTag = true;
}
}
DaveBTW, in software, hope and pray is not a viable strategy. (Luc Pattyn)Visual Basic is not used by normal people so we're not covering it here. (Uncyclopedia)
|
|
|
|
|
DaveyM69 wrote: so that shouldn't be the source of your problem.
unless an exception gets thrown, and got swallowed and/or not reported in the post.
Anyway your using construct fixes that.
The file data itself is the only large thing I could spot, nothing else could exhaust memory with less than 25 files, except just maybe the very first Directory.GetAllFiles().
Luc Pattyn [Forum Guidelines] [My Articles]
- before you ask a question here, search CodeProject, then Google
- the quality and detail of your question reflects on the effectiveness of the help you are likely to get
- use the code block button (PRE tags) to preserve formatting when showing multi-line code snippets
|
|
|
|
|
Luc Pattyn wrote: except just maybe the very first Directory.GetAllFiles().
There would have to be a LOT of files for that to happen, but it may be possible.
Luc Pattyn wrote: unless an exception gets thrown
I didn't look at the FilStreamEx class, but I didn't see any try/catch or usings in the ID3Info or ID3v1 classes I looked at, so it's a possibility if they're not caught elsewhere.
DaveBTW, in software, hope and pray is not a viable strategy. (Luc Pattyn)Visual Basic is not used by normal people so we're not covering it here. (Uncyclopedia)
|
|
|
|
|
as far as that article goes there isn't a single "try" keyword in the entire solution.
there are several static lists/collections but I doubt they could really grow large with only 25 files.
I think I got it: there is a MemoryStream "MStream" inside FileStreamEx, it never gets Disposed().
Luc Pattyn [Forum Guidelines] [My Articles]
- before you ask a question here, search CodeProject, then Google
- the quality and detail of your question reflects on the effectiveness of the help you are likely to get
- use the code block button (PRE tags) to preserve formatting when showing multi-line code snippets
|
|
|
|
|
Good find. I got too tired to be looking over countless classes that weren't written by the OP and went to bed!
DaveBTW, in software, hope and pray is not a viable strategy. (Luc Pattyn)Visual Basic is not used by normal people so we're not covering it here. (Uncyclopedia)
|
|
|
|
|
|
Please show the code again, the way it looks now.
Luc Pattyn [Forum Guidelines] [My Articles]
- before you ask a question here, search CodeProject, then Google
- the quality and detail of your question reflects on the effectiveness of the help you are likely to get
- use the code block button (PRE tags) to preserve formatting when showing multi-line code snippets
|
|
|
|
|
int i = 0,duration, year;
string artist, title, album, bandname, genre, path;
string delSql = "DELETE * FROM Library";
musLibConn.ConnectionString = @"Provider=Microsoft.Jet.OLEDB.4.0;Data Source=C:\Program Files\GeneSys\Library.mdb";
OleDbCommand deleteLib = new OleDbCommand(delSql, musLibConn);
musLibConn.Open();
if (tbMusPath.Text != "")
{
utility util = new utility();
filepaths = Directory.GetFiles(util.getLibPath(), "*.mp3", SearchOption.AllDirectories);
deleteLib.ExecuteNonQuery();
pBar.Visible = true;
foreach(string str in filepaths)
{
lblPath.Text = str;
ID3Info CurrSong = new ID3Info(filepaths[i], true);
artist = CurrSong.ID3v2Info.GetTextFrame("TPE1");
title = CurrSong.ID3v2Info.GetTextFrame("TIT2");
album = CurrSong.ID3v2Info.GetTextFrame("TALB");
string insSql = "INSERT INTO Library(Artist,Title,Album,Path)" +
"VALUES('" + artist.Replace("'", "''") + "','" + title.Replace("'", "''") + "','" +
album.Replace("'", "''") + "','" + str.Replace("'", "''") + "')";
using (OleDbCommand insertLib = new OleDbCommand(insSql, musLibConn))
{
insertLib.ExecuteNonQuery();
}
i++;
}
pBar.Visible = false;
lblNum.Text = Convert.ToString(filepaths.Length);
util.setNoFiles(filepaths.Length);
|
|
|
|
|
Thanks.
Could you tell us approximately how many files are there, and what would be their typical size.
Are there any exceptions thrown while the app runs (I mean before the OutOfMemory of course)?
[ADDED]
Did you check for any errors in the ID3Info class? There is a Clear() method, and Count and List properties that would return the number and the array or errors.
[/ADDED]
Luc Pattyn [Forum Guidelines] [My Articles]
- before you ask a question here, search CodeProject, then Google
- the quality and detail of your question reflects on the effectiveness of the help you are likely to get
- use the code block button (PRE tags) to preserve formatting when showing multi-line code snippets
|
|
|
|
|
sorry for taking so long.
RIght now there are approximately 2000 files, with the potential for 40,000 at between 3 and 6 mb.
The program runs fine up until I build the database and get the memory exception.
I will try your other suggestion about the filestream class.
|
|
|
|
|
Hi,
the FileStreamEx class has a ReadText method that creates a MemoryStream but never disposes of it.
That is bad. I suggest you replace line 90
return GetEncoding(TEncoding).GetString(MStream.ToArray());
by
string result=GetEncoding(TEncoding).GetString(MStream.ToArray());
MStream.Dispose();
return result;
It is likely that will dramatically improve the situation.
Luc Pattyn [Forum Guidelines] [My Articles]
- before you ask a question here, search CodeProject, then Google
- the quality and detail of your question reflects on the effectiveness of the help you are likely to get
- use the code block button (PRE tags) to preserve formatting when showing multi-line code snippets
|
|
|
|
|
doing this throws the same exception but from the binaryFrame.cs
|
|
|
|
|
You have the failing app in front of you, no one else has, so you should try and provide all available information, such as:
1. is it better, i.e. are more files being handled than before, how many?
2. everything you have about the exception, including the stack traceback including line numbers.
Anyway, I'm calling it a day now.
Luc Pattyn [Forum Guidelines] [My Articles]
- before you ask a question here, search CodeProject, then Google
- the quality and detail of your question reflects on the effectiveness of the help you are likely to get
- use the code block button (PRE tags) to preserve formatting when showing multi-line code snippets
|
|
|
|
|
Sorry for the vagueness
It is handling less than before, doesnt appear to be handling more than 2 or three now.
Same exception, different place. BinaryFrame.cs Line 31
28 internal BinaryFrame(string FrameID, FrameFlags Flags, FileStreamEx Data, int Length)
29 : base(FrameID, Flags)
30 {
31 _Data = Data.ReadData(Length);
32 }
StackTrace
at System.IO.FileStreamEx.ReadData(Int32 Length) in C:\Users\Jon Henry\Desktop\RoundButtons\ID3Class\ID3Class\FileStreamEx.cs:line 147
at ID3.ID3v2Frames.BinaryFrames.BinaryFrame..ctor(String FrameID, FrameFlags Flags, FileStreamEx Data, Int32 Length) in C:\Users\Jon Henry\Desktop\RoundButtons\ID3Class\ID3Class\Frames Classes\BinaryFrame.cs:line 31
at ID3.ID3v2.AddFrame(FileStreamEx Data, String FrameID, Int32 Length, FrameFlags Flags) in C:\Users\Jon Henry\Desktop\RoundButtons\ID3Class\ID3Class\ID3v2.cs:line 1120
at ID3.ID3v2.ReadFrames(FileStreamEx Data, Int32 Length) in C:\Users\Jon Henry\Desktop\RoundButtons\ID3Class\ID3Class\ID3v2.cs:line 896
at ID3.ID3v2.Load() in C:\Users\Jon Henry\Desktop\RoundButtons\ID3Class\ID3Class\ID3v2.cs:line 443
at ID3.ID3v2..ctor(String FilePath, Boolean LoadData) in C:\Users\Jon Henry\Desktop\RoundButtons\ID3Class\ID3Class\ID3v2.cs:line 77
at ID3.ID3Info..ctor(String FilePath, Boolean LoadData) in C:\Users\Jon Henry\Desktop\RoundButtons\ID3Class\ID3Class\ID3Info.cs:line 25
at JukeBox.frmOptions.btnBLib_Click(Object sender, EventArgs e) in C:\Users\Jon Henry\Documents\Visual Studio 2005\Projects\JukeBox\JukeBox\frmOptions.cs:line 116
at System.Windows.Forms.Control.OnClick(EventArgs e)
|
|
|
|
|
First, don't mix case in the same scope. Local variables are usually either camelCase, or PascalCase but doing both is very poor practice, most people use camelCase. Watch the types of your objects, calling ToStrings on strings shows a poor thought process. Also, don't update the GUI in your loop (lblPath.Text = path) it can be part of the problem and most likely will not update with any regularity.
I do not know if this will work but you can try the code below: (Fix any of my typing errors as I typed it in this window), I don't know if JET supports prepared parameters but if it does you can call command.Prepare() before the loop begins.
OleDbCommand command = new OleDbCommand();
command.Connection = connection;
command.CommandText = "INSERT INTO library(artist, title, album, path) VALUES (@artist, @title, @album, @path);"
command.Parameters.Add("@artist", OleDbType.VarChar);
command.Parameters.Add("@title", OleDbType.VarChar);
command.Parameters.Add("@album", OleDbType.VarChar);
command.Parameters.Add("@path", OleDbType.VarChar);
string sqlString = ";
foreach(string path in filePaths){
currentSong = new ID3Info(path, true); //Is this disposable? If so use using(currentSong = new ID3Info(path, true){
command.Parameters["@artist"].Value = currentSong.ID3v2Info.GetTextFrame("TPE1");
command.Parameters["@title"].Value = currentSong.ID3v2Info.GetTextFrame("TIT2");
command.Parameters["@album"].Value = currentSong.ID3v2Info.GetTextFrame("TALB");
command.Parameters["@path"].Value = path;
command.ExecuteNonQuery();
}
|
|
|
|
|
Hey folks,
I am very new to c# and have a small problem.
The problem :
My mainForm loads up in the CenterScreen position, if I open a new form(form2) it currently opens in CenterScreen position as well.
When form2 loads, form1 falls away.
What I'd like to achieve :
If I moved my mainForm before calling form2 up, I would like form2 to appear in the mainForm's last position.
Thanks in advance for any guidance in this matter.
Paulo.
|
|
|
|
|
If form2 is insanciated from form1, then you can set form2's location property after creating form2 but before calling it's show method. Make sure form2's StartPosition is set to Manual.
If not, you could use the project's settings to save the position of form1, and retrieve those settings in form2's constructor.
DaveBTW, in software, hope and pray is not a viable strategy. (Luc Pattyn)Visual Basic is not used by normal people so we're not covering it here. (Uncyclopedia)
|
|
|
|
|
Hi Dave,
Thanks for the reply.
Do you think it would be possible if you could write some example code for me, just to get a better understanding of the solutions you have supplied?
Sorry, like I said I am very new to C# and programming in general. :/
Thanks again for the reply.
|
|
|
|
|
This will help you more. Find some keywords here and Google it, it will give you some code, and explain what you are doing.
You need to create an instance of Form2 from Form1. Then get Form1's position, and set Form2's position to the same. Finally, show Form2.
You should be able to get started with those steps.
The best way to accelerate a Macintosh is at 9.8m/sec² - Marcus Dolengo
|
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
