pdudits / soundlibs

Maven artifacts for Java Sound Libraries
87 stars 25 forks source link

Update bundles to also be JPMS modules for java versions 9 and above #14

Open axiopisty opened 4 years ago

axiopisty commented 4 years ago

Each artifact published to maven central, in addition to being valid OSGI bundles, should also be valid java modules. In other words, each bundle should also include a valid module-info.class.

pdudits commented 4 years ago

PRs for that are welcome!

axiopisty commented 4 years ago

I'm working on this, slowly. But I just wanted to call out here, before submitting a pull request, that doing this will require more work than simply moving files into a modular structure and adding the module-info files.

Specifically, soundlibs currently uses sun.misc.Perf, a JDK internal that was supposed to not be used by anything except the JVM. This class has been removed in Java 9.

The only reason soundlibs uses this JDK internal class is to calculate the number of elapsed microseconds since the JVM started. So, I'm going to have to make a change to the SunMiscPerfClock class to accomplish this some other way. My thought is to use java.time.Instant as a replacement, which has nanosecond precision. I don't know yet if this will introduce any bugs or other problems.

Source code for sun.misc.Perf can be reviewed here: https://github.com/frohoff/jdk8u-jdk/blob/master/src/share/classes/sun/misc/Perf.java#L521. Notice that the current implementation of SunMiscPerfClock is based on the highResCounter method which, according to the java docs:

The High Resolution Counter returns the number of ticks since the start of the Java virtual machine. The resolution of the counter is machine dependent and can be determined from the value return by the {@link #highResFrequency} method.

My takeaway from this is that the getMicroseconds method in the SunMiscPerfClock currently returns the number of elapsed microseconds since the JVM starts. It probably doesn't matter so much that this counter starts when the JVM starts as the library appears to use this as a timer or clock for the audio sequencer.

Here is the implementation I'm contemplating:

public class SunMiscPerfClock
implements JavaSequencer.Clock
{

    private static final Instant start = Instant.now();

    /** Retrieve system time in microseconds.

        @return the system time in microseconds
    */
    public long getMicroseconds()
    {
        Duration elapsed = Duration.between(start, Instant.now());
        return TimeUnit.MICROSECONDS.convert(elapsed.toNanos(), TimeUnit.NANOSECONDS);
    }
}

The only testing I will be able to do, prior to submitting a pull request, will be to use the jar files I produce locally in a different application I am working on that uses the soundlibs to see if my application works appropriately with the new version of this library, with these changes in it.

Please provide any feedback or suggestions if you have any.

Thanks.

axiopisty commented 4 years ago

Actually, I discovered the Perf class is still available in the java.base module in the jdk.internal package. So we don't need to make this change right now. But, I'm still curious what people think of this. Maybe we can test to see if this approach would work in order to remove the dependency on the jdk.internal implementation.

pdudits commented 4 years ago

I really cannot give much input here, my experience with JPMS is very limited.

However I'd like to point out method [System.nanotime](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/System.html#nanoTime()), which will cause less allocation overhead, and I think it's also monotonic -- value of Instant can return lower value than before, e. g. after time synchronization over network.

JCWasmx86 commented 3 years ago

I tried it today to make those jars modules.

My observations: -SunMiscPerfClock class is using sun. package => Solved as seen above -mp3spi.jar and vorbisspi.jar both contain the package javazoom.spi with the class PropertiesContainer -tritonus-share.jar and tritonus-all.jar contain the package `org.tritonus.share.andorg.tritonus.sampled.file.**` (This is quite obvious, but would make the start of the JVM fail, as each package may only be in one module)

I'm now trying to package mp3spi,vorbisspi,jlayer,tritonus-share and jorbis as JPMS-modules in my repo jpms_sound