7

I have a recursive method that steps through a large directory containing thousands of music files. It adds a music file to an observableList<> each time the extension meets the criteria. The list is hooked into a TableView<> in a different thread prior to the recursive method executing so that the user can see the files being added to the TableView<> in real time.

The problem is I know very little about how to manage memory in java and think I might be getting in the way of garbage collection. The recursive method eats up almost 6 GB of ram after around 3,000 songs and then begins to ignore files that it should be able to read. Furthermore, after it has 'finished' stepping through the directory structure, the ram does not reduce, (i.e. the stack from the recursive method is not being destroyed and I think all the objects that are referenced are still in heap memory).

It goes further.. I export the playlist to an XML file and close the program. When I relaunch it, the memory is completely reasonable, so i know its not the large list containing the files, it must have something to do with the recursive method.

Here's the recusive method located in the music handler:

 /**
 * method used to seek all mp3 files in a specified directory and save them
 * to an ObservableArrayList
 * 
 * @param existingSongs
 * @param directory
 * @return
 * @throws FileNotFoundException
 * @throws UnsupportedEncodingException
 */
protected ObservableList<FileBean> digSongs(ObservableList<FileBean> existingSongs,
        File directory) throws FileNotFoundException,
        UnsupportedEncodingException {
    /*
     * Each directory is broken into a list and passed back into the digSongs().
     */
    if (directory.isDirectory() && directory.canRead()) {

        File[] files = directory.listFiles();
        for (int i = 0; i < files.length; i++) {
            digSongs(existingSongs, files[i]);
        }

        /*
         * if a file is not a directory, then is it checked to see if it's
         * an mp3 file
         */
    } else if (directory.getAbsolutePath().endsWith(".mp3") 
            || directory.getAbsolutePath().endsWith(".m4a")
            ) {
        FileBean songBean = new FileBean(directory).getSerializableJavaBean();

        existingSongs.add(songBean);

        songBean.getPlayer().setOnReady(new OnMediaReadyEvent(songBean));
        songBean.getPlayer().setOnError(new OnMediaPlayerStalled(existingSongs, songBean));

        /*
         * if it's not a directory or mp3 file, then do nothing
         */
    } else {

        return existingSongs;

    }

    return existingSongs;
}

Here is the listener for the MediaPlayer used to read thr ID tags if possible, this is also located in the music handler

/**
 * This class will populate the FileBean metaData after the MediaPlayer's
 * status has been changed to READY. Uses the FileBean's setter methods so
 * that they will be picked up by the XMLEncoder. This allows the use of the
 * Media's ID3v2 tag reading abilities. If tags are not read due to
 * incompatibility, they are not changed.
 * 
 * This step is computationally expensive but should not need to be done
 * very often and it saves a ton of memory during normal use. Setting the 
 * Media and MediaPlayer objects to null make this run much faster and uses
 * less memory
 * 
 * @author Karottop
 *
 */
protected class OnMediaReadyEvent implements Runnable {
    private FileBean fileBean;

    public OnMediaReadyEvent(FileBean fileBean) {
        this.fileBean = fileBean;
    }

    @Override
    public void run() {
        String songName = null;
        String album = null;
        String artist = null;
        double duration = 0.0;
        try{
            // Retrieve track song title
            songName = (String) fileBean.getMedia().getMetadata()
                    .get("title");

            // Retrieve Album title
            album = (String) fileBean.getMedia().getMetadata()
                    .get("album");

            // Retrieve Artist title
            artist = (String) fileBean.getMedia().getMetadata()
                    .get("artist");

            // Retrieve Track duration
            duration = fileBean.getMedia().getDuration().toMinutes();
        }catch(NullPointerException e){
            System.out.println(e.getMessage());
        }
        // Set track song title

        if (songName != null)
            fileBean.setSongName(songName);

        // Set Album title

        if (album != null)
            fileBean.setAlbum(album);

        // Retrieve and set Artist title

        if (artist != null)
            fileBean.setArtist(artist);

        // Set Track duration
        fileBean.setDuration(Double.parseDouble(
                XMLMediaPlayerHelper.convertDecimalMinutesToTimeMinutes(duration)));

        fileBean.setMedia(null);
        fileBean.setPlayer(null);

    }

}

Here is where I call the method in the controller for the FXML:

    public class LoadAllMusicFiles implements Runnable{

    private TableView<FileBean> tableView;

    public LoadAllMusicFiles(TableView<FileBean> tableView) {
        this.tableView = tableView;
    }   

    @Override
    public void run() {
        try {
            musicHandler.loadAllPlaylists();
            tableView.setItems(musicHandler.getMainPlaylist().getSongsInPlaylist());
            playlistTable.setItems(musicHandler.getPlaylists());

        } catch (FileNotFoundException e) {
            e.printStackTrace();
        } catch (NoPlaylistsFoundException e) {
            String title = "Mine for mp3s";
            String header = "No playlists were found.\n"
                    + "These are your mp3 mining options...";
            String content = "Do you want to import a single mp3\n"
                    + "or a folder containing many mp3s?\n\n"
                    + "**Note For large volumes of songs this may take a while.\n"
                    + "Grab some coffee or something..**";
            findNewSongs(title, header, content);
            // need to handle file not found exception in new thread
            tableView.setItems(musicHandler.getMainPlaylist().getSongsInPlaylist());
            playlistTable.setItems(musicHandler.getPlaylists());
            Platform.runLater(new SelectIndexOnTable(playlistTable, 0));
            tableView.getSelectionModel().selectFirst();

        }

    }

}

/**
 * The method will display an Alert box prompting the user to locate a 
 * song or directory that contains mp3s
 * 
 * The parameters passed is the text the user will see in the Alert box.
 * The Alert box will come with 3 new buttons: 1)Single mp3, 2)Folder of mp3s
 * and 3)Cancel. If the user selects the first button they will be
 * presented with a FileChooser display to select a song. If they press
 * the second button, the user will be prompted with a DirectoryChooser
 * display. The third button displays nothing and closes the Alert box.
 * 
 * The following outlines where each parameter will be displayed in the
 * Alert box
 * 
 * title: very top of the box in the same latitude as the close button.
 * header: inside the Alert box at the top.
 * content: in the middle of the box. This is the best place to explain
 * the button options to the user.
 * @param title
 * @param header
 * @param content
 */
private void findNewSongs(String title, String header, String content){
    Alert importType = new Alert(AlertType.CONFIRMATION);
    importType.setTitle(title);
    importType.setHeaderText(header);
    importType.setContentText(content);

    ButtonType singleMp3 = new ButtonType("Single mp3");
    ButtonType folderOfmp3s = new ButtonType("Folder Of mp3s");
    ButtonType cancel = new ButtonType("Cancel", ButtonData.CANCEL_CLOSE);
    importType.getButtonTypes().setAll(singleMp3, folderOfmp3s, cancel);

    Optional<ButtonType> result = importType.showAndWait();
    if(result.get() == singleMp3){
        FileChooser fileChooser = new FileChooser();
        fileChooser.setTitle("Location of mp3s");
        ArrayList<String> extensions = new ArrayList<>();
        extensions.add("*.mp3");
        fileChooser.getExtensionFilters().add(
                new ExtensionFilter("Audio Files", getSupportedFileTypes()));

        File selectedFile = fileChooser.showOpenDialog(playBackButton.getScene().getWindow());

        if(selectedFile == null){
            return;
        }
        Thread findSongs = new Thread(new DigSongs(selectedFile.getAbsolutePath()));
        findSongs.start();

    }else if(result.get() == folderOfmp3s){
        DirectoryChooser fileChooser = new DirectoryChooser();
        fileChooser.setTitle("Location to mine for mp3s");

        File selectedFile = fileChooser.showDialog(playBackButton.getScene().getWindow());

        if(selectedFile == null){
            return;
        }
        Thread findSongs = new Thread(new DigSongs(selectedFile.getAbsolutePath()));
        findSongs.start();

    }else{
        return;
    }
}

public class DigSongs implements Runnable{
    String path;

    public DigSongs(String path) {
        this.path = path;
    }
    @Override
    public void run() {
        Platform.runLater(new UpdateLabel(digLabel, "loading..."));
        try {
            musicHandler.findNewSongs(path);

        } catch (FileNotFoundException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        } catch (UnsupportedEncodingException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        ObservableList<FileBean> songArray = musicHandler.getMainPlaylist().getSongsInPlaylist();
        Platform.runLater(new UpdateLabel(digLabel, "complete: " + songArray.size()));
    }

}

This method is located in the music handler and basically just calls the recursive method digSongs(ObservableList, File):

/**
 * This method will search for songs in a new directory and add them to the song list
 * in the main playlist
 * @param newDirectory
 * @return
 * @throws FileNotFoundException
 * @throws UnsupportedEncodingException
 */
public PlaylistBean findNewSongs(String newDirectory) 
        throws FileNotFoundException, UnsupportedEncodingException{
    PlaylistBean main = getMainPlaylist();
    File file = new File(newDirectory);

    // add new songs to existing main playlist
    digSongs(main.getSongsInPlaylist(), file);

    return main;
}

Guys, I know this is a lot of code and stuff to read. I just can't seem to find the answers I need on google. I suspect the problem has something to do with the reference being passed to the TableView<> but I honestly don't know. I hope someone can take the time to look. I'll post more code if anyone needs it

EDIT: FileBean class

package fun.personalUse.dataModel;

import java.io.File;
import java.io.FileNotFoundException;
import java.io.UnsupportedEncodingException;
import java.net.URLEncoder;
import java.util.Comparator;
import javafx.beans.property.SimpleStringProperty;
import javafx.scene.media.Media;
import javafx.scene.media.MediaPlayer;

/**
 * Data model for use with a media player. This object is intended to store
 * song data for 1 song
 * @author Karottop
 *
 */
public class FileBean implements Comparator<FileBean>, Comparable<FileBean>{
private File file;
private SimpleStringProperty location;
private SimpleStringProperty songName;
private SimpleStringProperty  album;
private SimpleStringProperty  artist;
private SimpleStringProperty  url;
private Media media;
private MediaPlayer player;
private SimpleStringProperty  duration;

/**
 * inserts default or null values for every field. This constructor
 * should be used when making a serializable FileBean. setters should
 * be used to initialize the object
 */
public FileBean(){
    media = null;
    file = null;
    location = new SimpleStringProperty();
    songName = new SimpleStringProperty();
    album = new SimpleStringProperty();
    artist = new SimpleStringProperty();
    url = new SimpleStringProperty();

    /**
     *  must initialize with a number because this field will be called
     *  before the MediaPlayer's status has changed which would cause a 
     *  null pointer exception to be thrown if not initialized
     */
    duration = new SimpleStringProperty("0.0");
}

/**
 * Initializes the file bean using a file
 * @param file
 * @throws FileNotFoundException
 * @throws UnsupportedEncodingException
 */
public FileBean(File file) throws FileNotFoundException, UnsupportedEncodingException{
    location = new SimpleStringProperty();
    songName = new SimpleStringProperty();
    album = new SimpleStringProperty();
    artist = new SimpleStringProperty();
    url = new SimpleStringProperty();

    /**
     *  must initialize with a number because this field will be called
     *  before the MediaPlayer's status has changed which would cause a 
     *  null pointer exception to be thrown if not initialized
     */
    duration = new SimpleStringProperty("0.0");
    this.file = file;
    location.set(file.getAbsolutePath().replace("\\", "/"));

    /*
     * encode all special characters.
     * URLEncoder puts a '+' where a ' ' is so change all '+' to encoded space '%20'.
     */
    url.set(URLEncoder.encode(location.get(), "UTF-8").replace("+", "%20"));

    /*
     * Could not easily figure out how to set an action event for when the Media
     * object is done loading. Using the MediaPlayer status change event instead.
     * Looking for a better option
     */
    media = new Media("file:///" + url.get());
    this.player = new MediaPlayer(media);
    setDefaultSongNameAndArtist();
}

public FileBean(String absolutePath) throws FileNotFoundException, UnsupportedEncodingException{
    this(new File(absolutePath));
}

/**
 * This method uses the parent directory strucutre to guesstimate
 * what the song name, artist and album name is. a '?' is appended at the
 * end of each item to indicate this is a guessed value
 * 
 * media file that do not adhere to the following directory structure 
 * will not be named correctly:
 * 
 * pathToMedia/Artist/Album/song
 */
private void setDefaultSongNameAndArtist(){
    String[] songLocation = getLocation().split("/");
    String[] songFragment = songLocation[songLocation.length - 1].split("[.]");
    setSongName(songFragment[0]);

    setAlbum(songLocation[songLocation.length - 2] + "?");
    setArtist(songLocation[songLocation.length - 3] + "?");

}



/**
 * @return the player
 */
public MediaPlayer getPlayer() {
    return player;
}

/**
 * @param player the player to set
 */
public void setPlayer(MediaPlayer player) {
    this.player = player;
}

/**
 * @return the duration
 */
public double getDuration() {
    return Double.parseDouble(duration.get());
}



/**
 * @param duration the duration to set
 */
public void setDuration(double duration) {
    this.duration.set(String.format("%.2f", duration));
}



/**
 * @return the album
 */
public String getAlbum() {
    return album.get();
}



/**
 * @param album the album to set
 */
public void setAlbum(String album) {
    this.album.set(album);
}



/**
 * @return the artist
 */
public String getArtist() {
    return artist.get();
}



/**
 * @param artist the artist to set
 */
public void setArtist(String artist) {
    this.artist.set(artist);
}



/**
 * @return the media
 */
public Media getMedia() {
    return media;
}



/**
 * @param media the media to set
 */
public void setMedia(Media media) {
    this.media = media;
}



/**
 * @return the url
 */
public String getUrl() {
    return url.get();
}


/**
 * @param url the url to set
 */
public void setUrl(String url) {
    this.url.set(url);
}


/**
 * @return the file
 */
public File getFile() {
    return file;
}

/**
 * @param file the file to set
 */
public void setFile(File file) {
    this.file = file;
}

/**
 * @return the location
 */
public String getLocation() {
    return location.get();
}

/**
 * @param location the location to set
 */
public void setLocation(String location) {
    this.location.set(location);
}

/**
 * @return the name
 */
public String getSongName() {
    return songName.get();
}

/**
 * @param name the name to set
 */
public void setSongName(String name) {
    this.songName.set(name);
}

/**
 * returns the songName property
 * @return
 */
public SimpleStringProperty songNameProperty(){
    return songName;
}

/**
 * returns the artist property
 * @return
 */
public SimpleStringProperty artistProperty(){
    return artist;
}

/**
 * returns the album property
 * @return
 */
public SimpleStringProperty albumProperty(){
    return album;
}

/**
 * returns the duration property
 * @return
 */
public SimpleStringProperty durationProperty(){
    return duration;
}

/**
 * Creates a serializable copy of this object
 * by using it's setters. The purpose of this
 * method is so that the FileBean objects can
 * be exported to an XML
 * @return
 */
public FileBean getSerializableJavaBean(){
    FileBean temp = new FileBean();
    temp.setAlbum(this.getAlbum());
    temp.setArtist(this.getArtist());
    temp.setDuration(this.getDuration());
    temp.setFile(this.getFile());
    temp.setLocation(this.getLocation());
    temp.setMedia(this.getMedia());
    temp.setPlayer(player);
    temp.setSongName(this.getSongName());
    temp.setUrl(this.getUrl());

    return temp;
}

/**
 * Method used to return a fully populated FileBean after decoded from XML.
 * @return
 */
public FileBean getFullFileBean(){

    try {
        return new FileBean(new File(getLocation()));
    } catch (FileNotFoundException | UnsupportedEncodingException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
        FileBean temp = new FileBean();
        temp.setLocation("error");
        return temp;
    }
}

/**
 * Returns are string in the following format:
 * 
 * [song name], [artist name], [album name]
 */
@Override
public String toString(){
    return String.format("%s, %s, %s", getSongName(), getArtist(), getAlbum());
}

/**
 * uses FileBean.toSting().compareTo(this.toString())   to determine if the two
 * beans are equal
 */
@Override
public boolean equals(Object fileBean){
    FileBean newBean = (FileBean)fileBean;
    return newBean.toString().compareTo(this.toString()) == 0;
}


/**
 * Uses the String.compare() to order FileBeans based on their absolute path
 */
@Override
public int compareTo(FileBean bean) {
    if(this.getLocation().compareTo(bean.getLocation()) > 0){
        return 1;
    }else if(this.getLocation().compareTo(bean.getLocation()) < 0){
        return -1;
    } else{
        return 0;
    }
}

/**
 * uses the compareTo method to compare two files beans.
 * 
 * This method uses the String.compare() to order FileBeans
 * based on their absolute path
 */
@Override
public int compare(FileBean bean1, FileBean bean2) {
    // TODO Auto-generated method stub
    return bean1.compareTo(bean2);
}


}
MarsTwo
  • 95
  • 6
  • 7
    It may help to [profile](http://stackoverflow.com/q/2064427/230513) your application during use. – trashgod Aug 26 '16 at 17:14
  • 1
    @trashgod I've never heard of profiling applications, thats a great link, Ill look into it – MarsTwo Aug 26 '16 at 17:18
  • is a `FileBean` instance a _heavy_ object? When loading from the XML, does the load call `songBean.getPlayer()...`? [This](http://stackoverflow.com/questions/258120/what-is-the-memory-consumption-of-an-object-in-java) might help. – Andrew S Aug 26 '16 at 17:45
  • Are you loading the contents of the files into memory? – Jimmy T. Aug 26 '16 at 17:52
  • @Andrew S FileBean is a class I created. I can post the source code for that too if it'll help. It contains a MediaPlayer and Media object fields that are used to read the ID tags on the media Files. I try to de-reference them as much as possible but they very well may be part of the problem. – MarsTwo Aug 26 '16 at 18:21
  • @Jimmy T: No not explicitly. I am instantiating a MediaPlayer and Media object in the constructor tho which maybe loads the data into memory, not sure. Ill post that class just in case it helps. Kind of blew the top off trying to post a modest amount of code anyways so whats a few hundred extra lines – MarsTwo Aug 26 '16 at 18:25
  • @Andrew S To answer your question about the XML, no the XML populates the song information fields that were discovered during mining, but does not instantiate the Media or MediaPlayer fields. Those are only instantiated when the application mines for mp3s. Are you thinking those fields are what's eating up the memory? If so how would you suggest to fix it? – MarsTwo Aug 26 '16 at 18:38
  • 1
    Can you try caling `fileBean.getPlayer().dispose()` before calling `fileBean.setPlayer(null)`? I think that's necessary to free the resources of the `MediaPlayer` – Jimmy T. Aug 26 '16 at 22:26
  • You know, I didn't know that method existed, I'll give it a try right now – MarsTwo Aug 26 '16 at 23:33
  • @Jimmy T: `fileBean.getPlayer().dispose()` actually did make a difference. The application was unfortunately quicker to max out the RAM and it ran slower, but it did completely parse the music directory and then ran with much less RAM after finishing the mp3 mining. That was a great suggestion, perhaps the high memory use is just something I'll have to deal with. – MarsTwo Aug 26 '16 at 23:50
  • Can you post your JVM configuration specially if you are using any memory config params to JVM? What you see might be because of having lots of started threads. I suggest first you profile your code and focus on seeing number of threads in your application. You can try using ThreadPools to control the number of overall threads as they have notable overhead on JVM. You should note that unless you are assigning too much memory (more than your ram) to the JVM and it starts using your swap space, the program doesn't get slow. It simply runs out of memory and exits with error. – vizog Aug 30 '16 at 20:06
  • What if you try to change your approach to first get list of songs, then read their ID tags? So your `digSongs` would change to something like `ObservableList digSongs( ... )` where you would only return list of paths to songs and then run another method that would read ID tags of that songs? ... Also, why do you even need MediaPlayer instance for each song? I guess (from documentation) just to make sure Media reading has been done. But after that? I guess after calling `setDefaultSongNameAndArtist` you should dispose it (as Jimmy suggested) – Buksy Aug 31 '16 at 06:09
  • @vizog I've since run the application a few more times after implementing Jimmy T's suggestion to use `MediaPlayer.dispose()` and the memory actually did quite well. I'm not sure what happened the first time, but I believe that was the key. It still runs much slower as I think `dispose` is a taxing method. Perhaps ThreadPools as you suggested may help. I haven't profiled an application or used ThreadPools before so there will be a learning curve. I'll post again once I figure it out – MarsTwo Aug 31 '16 at 14:40
  • @Buksy I need a MediaPlayer for each song for two reasons – MarsTwo Aug 31 '16 at 14:42
  • 1) Many playable songs have the .m4a extension which is supported by the the java `MediaPlayer`, unfortunately the Apple lossless encoding format also has an .m4a file extension which is not supported by the java `MediaPlayer`. The only way I can tell the difference is if the `MediaPlayer` changes it's status to `ERROR` in which case I remove the song from the list. 2) I Haven't written a custom method to read ID tags so I'm using the MediaPlayer to do that (if it can). It also is necessary to tell me the duration of the song – MarsTwo Aug 31 '16 at 14:52
  • I really appreciate everyone's help so far – MarsTwo Aug 31 '16 at 14:59

1 Answers1

2

It is almost always a bad idea to use File.listFiles() because it eagerly allocates an array of files which may be very memory consuming.

So recursive digSongs method may produce significant peak memory usage (or even lead to OutOfMemoryError).

Take a look at Files.walkFileTree(...). It is a great memory efficient solution for directory traversal.

vsminkov
  • 10,912
  • 2
  • 38
  • 50