Simple audio playback system

Question

I am trying to optimize my game written in Java and I used the VisualVM launcher for profiling and monitoring my game. What caught me by surprise was that my basic utility to play sound effects was using the most CPU in my game.

I'd really appreciate some tips on improvements which I can implement, because I really don't think that this basic utility for playing sound effects should utilize that much processing power!

Here is the relevant method for playing audio:

public static synchronized void playSound(final AudioInputStream inputStream, int priority) {
    switch(priority){
    default:
    case LOW_PRIORITY:
        if(nThreads>= MAX_THREADS>>1) return;
        break;
    case NORMAL_PRIORITY:
        if(nThreads>= MAX_THREADS) return;
        break;
    case HIGH_PRIORITY:
        break;
    }
    new Thread(() -> {
        nThreads++;
        try {
            Clip clip = AudioSystem.getClip();
            inputStream.reset();
            clip.open(inputStream);
            clip.start();
            while(clip.getMicrosecondPosition() < clip.getMicrosecondLength());
        } catch (Exception e) {
            e.printStackTrace();
        }
        nThreads--;
    }).start();
}

Just for reference here is the whole class:

package foo.bar.audio;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.net.URL;

import javax.sound.sampled.AudioInputStream;
import javax.sound.sampled.AudioSystem;
import javax.sound.sampled.Clip;
import javax.sound.sampled.UnsupportedAudioFileException;

public class BasicSoundEffect {

    public static final int MAX_THREADS = 20;

    public static final int HIGH_PRIORITY   = 2;
    public static final int NORMAL_PRIORITY = 1;
    public static final int LOW_PRIORITY    = 0;

    public static AudioInputStream SFX_BOSS;
    public static AudioInputStream SFX_HIT;
    public static AudioInputStream SFX_HURT;
    public static AudioInputStream SFX_LOSE;
    public static AudioInputStream SFX_PICKUP;
    public static AudioInputStream SFX_SHOOT;
    public static AudioInputStream SFX_WIN;

    static{
        try {
            SFX_BOSS    = createReusableAudioInputStream(BasicSoundEffect.class.getResource("/boss.wav"));
            SFX_HIT     = createReusableAudioInputStream(BasicSoundEffect.class.getResource("/hit.wav"));
            SFX_HURT    = createReusableAudioInputStream(BasicSoundEffect.class.getResource("/hurt.wav"));
            SFX_LOSE    = createReusableAudioInputStream(BasicSoundEffect.class.getResource("/lose.wav"));
            SFX_PICKUP  = createReusableAudioInputStream(BasicSoundEffect.class.getResource("/pickup.wav"));
            SFX_SHOOT   = createReusableAudioInputStream(BasicSoundEffect.class.getResource("/shoot.wav"));
            SFX_WIN     = createReusableAudioInputStream(BasicSoundEffect.class.getResource("/win.wav"));
        } catch (Exception e) {
            e.printStackTrace();
        }
    }

    private static AudioInputStream createReusableAudioInputStream(URL url) 
            throws IOException, UnsupportedAudioFileException
        {
            AudioInputStream ais = null;
            try
            {
                ais = AudioSystem.getAudioInputStream(url);
                byte[] buffer = new byte[1024 * 32];
                int read = 0;
                ByteArrayOutputStream baos = 
                    new ByteArrayOutputStream(buffer.length);
                while ((read = ais.read(buffer, 0, buffer.length)) != -1)
                {
                    baos.write(buffer, 0, read);
                }
                AudioInputStream reusableAis = 
                    new AudioInputStream(
                            new ByteArrayInputStream(baos.toByteArray()),
                            ais.getFormat(),
                            AudioSystem.NOT_SPECIFIED);
                return reusableAis;
            }
            finally
            {
                if (ais != null)
                {
                    ais.close();
                }
            }
        }

    private static volatile int nThreads = 0;

    public static synchronized void playSound(final AudioInputStream inputStream, int priority) {
        switch(priority){
        default:
        case LOW_PRIORITY:
            if(nThreads>= MAX_THREADS>>1) return;
            break;
        case NORMAL_PRIORITY:
            if(nThreads>= MAX_THREADS) return;
            break;
        case HIGH_PRIORITY:
            break;
        }
        new Thread(() -> {
            nThreads++;
            try {
                Clip clip = AudioSystem.getClip();
                inputStream.reset();
                clip.open(inputStream);
                clip.start();
                while(clip.getMicrosecondPosition() < clip.getMicrosecondLength());
            } catch (Exception e) {
                e.printStackTrace();
            }
            nThreads--;
        }).start();
    }
}

Answer 1

Using volatile is no longer a recommended way of handling memory access. A much better way would be to use an AtomicInteger. It has increment and decrement methods to help.

Additionally, you still have race condition in your method even though it is synchronized.... you could have a situation where the nThreads changes (up or down) during execution of the playSound method because the Runnable is running in a different method.

To avoid the race condition, just pull the nThreads in to a local variable once and reference that.

As for your performance problem, it is here:

            while(clip.getMicrosecondPosition() < clip.getMicrosecondLength());

That code.... is horrible... really horrible.

What you do is spin-loop with a CPU at 100% utilization until the sound finishes. You need to do some blocking (or sleeping) until the sound completes.

I would rewrite that loop, and the nThreads something like (note, you will have to handle an InterruptedException somewhere):

AtomicInteger nThreads = new AtomicInteger(0);

// note, not synchronized
public static void playSound(final AudioInputStream inputStream, int priority) {
    final int tcount = nThreads.get();
    switch(priority) {
    default:
    case LOW_PRIORITY:
        if(tcount >= MAX_THREADS >> 1) {
            return;
        }
        break;
    case NORMAL_PRIORITY:
        if(tcount >= MAX_THREADS) {
            return;
        }
        break;
    case HIGH_PRIORITY:
        break;
    }

    new Thread(() -> {
        nThreads.incrementAndGet();
        try {
            Clip clip = AudioSystem.getClip();
            inputStream.reset();
            clip.open(inputStream);
            clip.start();

            final long length = clip.getMicrosecondLength();
            long remaining = length - clip.getMicrosecondPosition(); 
            while(remaining > 0) {
                Thread.sleep(remaining);
                remaining = length - clip.getMicrosecondPosition();
            }
        } catch (Exception e) {
            e.printStackTrace();
        } finally {
            nThreads.decrementAndGet();
        }
    }).start();
}