Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
axiopisty opened this issue Apr 8, 2020 · 5 comments
Open

Comments

@axiopisty
Copy link
Contributor

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
Copy link
Owner

pdudits commented May 21, 2020

PRs for that are welcome!

@axiopisty
Copy link
Contributor Author

axiopisty commented Jun 14, 2020

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
Copy link
Contributor Author

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
Copy link
Owner

pdudits commented Jun 16, 2020

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

However I'd like to point out method System.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
Copy link

JCWasmx86 commented Apr 19, 2021

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.** and org.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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants