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

Build Cleanup - Using static classpaths rather than dynamic ones. #349

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

curttasker
Copy link
Member

Why I'm trying this

I've never been fully convinced that Anathema's dynamic classpath scheme using reflection was the way to go. I figured I'd try to do it "normally", and see what you thought of it.

There's still room for improvement, but I wanted you to see the general direction of my thoughts before I went any farther.

Build Cleanup

I started by getting rid of all the reflection code we use to build the classpath. This included deleting all of the files in :Anathema. I then took :Platform_Swing/Anathema.java, moved it to :Anathema, and gave it a static main method. This is now the main class of Anathema (which incidentally fixes the menu title issue in #348). I made :Anathema depend upon :Platform_Swing (it has to now).

:Anathema/build.gradle now uses gradle's runtime configurations to build a classpath, which is then put into the manifest.mf file for :Anathema:jar. This replaces the old reflection code used to build the classpath dynamically. Note that it is impossible to simply say include all jar files from this folder in a manifest file; You must explicitly denote each file.

I also threw in my version of the copyAllDependencies code while I was mucking around in here. It seems to work well, but @UrsKR's version worked equally well. (See my email comments on picking one over the other).

JavaFX Exclusions

With Java 8 and newer, JavaFX classes (jfxrt.jar) will be on the default runtime classpath. With Java 1.7.0_06 and higher, jfxrt.jar is included as part of the JRE, but you have to manually specify this file on the java runtime classpath.

Currently Anathema bundles this jar inside the JRE for Win/Mac, and we explicitly exclude it being included as part of our runtime dependencies to avoid duplication.

I ran into a minor snag with this method and the new classpaths: There is no way to reference JAVA_HOME inside a manifest file, so you're unable to add jfxrtr.jar to the classpath.

This called my attention to an edge case. While we can enforce a minimum JRE version of 1.7 for people running the Zip distribution, we can't enforce 1.7.0_21. Since JavaFX was only included on 1.7.0_06 and newer, its possible for users with older Java 7 installs to attempt to run Anathema through the jar file, and it will fail because there is no jfxrt.jar on their system.

The new JavaFX bundling tasks should alleviate this, but until we switch over to it, we need a workaround. I decided on removing our exclusions for jfxrt.jar from our dependencies; I compensate by adding the same inclusion to our bundled JREs. Thus we're still only shipping one copy of this jar, its location has just moved.

Possible problems

I didn't see any immediate issues with our update system, but you'd know better than I would. Because the classpath is now stored in the manifest file of :Anathema/Anathema.jar, if any new jar files get added to the project, then this jar file needs to be regenerated and pushed out with the update. This should happen automatically, as the Anathema.jar file should be regenerated every time the classpath changes.

Final Thoughts

Everything seems to be running just as it was before. I didn't run any benchmarks, but I suspect Anathema will launch slightly faster now, due to the lack of dynamic classpath code.

Anyways, let me know what your thoughts are on using this approach.

@ghost ghost assigned curttasker Jul 23, 2013
@UrsKR
Copy link
Member

UrsKR commented Jul 23, 2013

There were two reasons for the dynamic classpath, the update system and (replacing the) plugin system.
Well, and I never did much with classloading and wanted to try.

The latter reason is obsolete. We now support loading custom data via the repository and there are no external contributions anyway. For that reason alone, we could drop it.
The second reason is more complicated, as I don't think the update system will work with this change.
The current bootloader determines the directory to load .jars from via the properties file in the install directory. That file is updated by the update system, and I can't see another way to do it with the current update system.

I'm not too hot on having auto-updates, but some people (you among them?) were, and I don't see the need to take this away from them.
There are other advantages though, mainly that the system conforms to expectations this way, and we could probably simplify major parts of the build logic if we go this route.

And we could always include another updater: Maybe you can replace .jars at runtime, or maybe we could do the update-thingy before the main application starts and replace the jars before anything happens.
Maybe we could use JavaFX webstart for it?

I'd love your opinion on this.

@curttasker
Copy link
Member Author

Keeping Anathema Up To Date

I don't think I was ever totally behind in-app updating. I certainly don't mind software that has an update feature, and the easier it is, the more likely I am to update the software. Do we have any metrics on number of people using the system vs normal downloads?

That said, I personally have not used used Anathema's update feature outside of a few tests. When dealing with my "official" repository, I use a manually downloaded copy of Anathema, and I only update it a few times a year.

As I recall, one of the reasons people were clamoring for an update system was for better notification of the fact that there is an update. Currently you can keep track of updates by subscribing to the Google+ account, subscribing to the anathema.github.io RSS feed, monitoring the forums daily for an update post, or using the in-app update system daily. To further expand our ability to let people know there's an update, I'd like to see a preferences option to check for updates on startup (checked by default), regardless of what update method we use.

From personal experience, Anathema users I've dealt with just download Anathema once and almost never update it. They're new to Exalted, they endlessly play with Anathema during character generation, but after they export their final PDF it goes un-used for weeks. It was like pulling teeth to get them to update their copy of Anathema, even with the in-app update option. Often I'd find out they'd done several updates at my request, but had never run Anathema between updates.

Even assuming someone's running Anathema weekly to track XP and story notes, that doesn't mean they're necessarily going to want to update twice a month. Updates introduce new features and fix bugs, but they also generate bugs occasionally. Personally, unless the changelog lists something that affects me in a major way, I'm not likely to update infrequently used software like Anathema.

Updates and the Default ClassLoader

You're definitely right, the update system would not work at all with this method, I hadn't fully read all the documentation on the default ClassLoader last night. Once you've loaded your jar files with the default ClassLoader, it doesn't offer a mechanism for unloading and reloading them.

Removing Updates

We do have a large quantity of code in place to handle the custom ClassLoader and update system. It works currently, and I hesitate to remove something that works unless there are clear benefits.

Benefits
  1. Having a classpath is standard, and as such we would conform to expectations
  2. Would remove a barrier preventing us from submitting the application to App Stores (windows 8, osx).
  3. Allows us to start codesigning our app (windows 8, osx)
  4. Faster launching of Anathema
  5. Cleaner build process
  6. Less code to maintain

As to another updater, there are always options. The easiest thing I can think of is if we shipped a separate executable that could update anathema. Doing it as part of the launch process will always involve a custom ClassLoader.

JavaFX

I'll eventually tackle using the javafx bundling tasks, which will clean up a lot of our build process. We'll easily be able to generate a a runnable jar like we do now, self-contained standalone application packages to replace our launch4j/nsis/appbundler tasks, and webstart files.

I'm hesitant to start in on this immediately, as we have solid working build code already, but it may be worth doing before 3E.

@UrsKR
Copy link
Member

UrsKR commented Jul 23, 2013

If the ruckus back then really was about expectations only, I never read our users worse. I was convinced they were clamoring for an automated update system because replacing Anathema was an issue - after all, you have to remove the existing install first, lest versions interfere with each other.

Moving the Repo

And that would still be an issue now, coming to think of it. I'd say we should move the repository out of the install directory in the same breath, or shortly after, if we go this way. Which brings up other issues, namely that an external Repo would be picked up by any version of Anathema, and thus makes it harder to have several installs in parallel.
Which people do.
For whatever reason.
It's easy to write all of this up as separate stories, though, we should just keep it in mind. It could align nicely with the issues about the repo browser that T-T brought up recently.

Benefits

I'd like to see hard numbers before I believe in 4) and I don't know a thing about 2) and 3) but otherwise, I agree with the benefits.

Bundling via JavaFX

I can't see the benefit of commiting to a JavaFX build before we even have the system converted, let alone know the timespan between conversion and 3E. Once the migration is done, sure, but until then I believe your effort better spent elsewhere.

@UrsKR
Copy link
Member

UrsKR commented Jul 23, 2013

Two more things I forgot to answer:

  1. Here are our stats. Right now, for v5.1.2 it's an even split between full and update.
  2. Of course you'd have to run the updater separately. That's what I meant by "running the updater before the main application starts", I just phrased it a weird way.
    Thinking it through, that's probably the simplest thing:
  • Make a separate update application that's included in the build and downloads and installs a full version in a fully scripted way. Easiest done through a zip with OS specific contents including the Java dist, if it comes to that.
  • (Bonus) Add a "update available" notification to the program
  • (Gold Bonus) Add a button that quits Anathema and launches the updater to Anathema itself
  • (Platinum Bonus) Have the updater launch Anathema after the update.

@curttasker
Copy link
Member Author

Oh, there were definitely some people raising a ruckus over it, but there's always a few people like that in any community.

Manual Updating

As for requiring removing the existing install first, this is a non-issue on OSX, as manually installing the app automatically replaces the old code code, and you're able to run parallel copies (using the same repository) by just changing the application's name.

On Windows, doesn't the NSIS installer handle the update fine, or were we running into issues with it? For the zip installation, the entire thing is self contained, so it shouldn't be an issue.

Moving the Repo

For reference:

By default, Anathema is hardcoded to use ./repository unless overridden by a default or a custom setting. Thus, running the jar file directly uses ./repository, Windows uses ./repository (where . is the install directory location, we use NSIS to give that folder write permission inside program files), while OSX uses ~/Library/Application Support/Anathema, and Linux using the shell script uses ~/.repository

Remember that once a user selects a custom repository location in Preferences, it means that any copy of Anathema that runs on that user account will now use that location. We store the custom location in the Java Preferences, and this shared across all versions of anathema.

I do this, and I have copious backups of my repository for when I'm launching test copies of anathema. That said, I've rarely had an issue with a development copy of anathema screwing with my repository data, even when running 3E builds against my 2.5E repository.

We could pretty easily alter our preferences storage system to key repository locations to the location net.sf.anathema.Anathema is run from. This would allow a separate set of preferences per installed copy of Anathema.

JavaFX

You can use the JavaFX packaging tools even if you aren't actually using javaFX, here is oracle's argument for it.

@UrsKR
Copy link
Member

UrsKR commented Jul 23, 2013

I added some thoughts about replacing the updater to my previous post while you were writing. Sorry for the confusion.

Does the NSIS installer work around existing Repos? I tend to forget, I only use it for testing and usually launch Anathema from the IDE. What do you mean by "Zip installation is self contained"? That wouldn't solve repo transition issues, would it?

Regarding the repo move, yes, it came to my mind while I was writing about it earlier that the issue already exists.
Regarding preferences, I am all in favor of doing just that, storing them in a properties or JSON file. Global storage bugs me just a much as static variables, and I wanted to do this very thing when I get there for the FX migration, since it will prompt a near-complete rewrite of the preferences system anyway.

@curttasker
Copy link
Member Author

By "self contained zip install", I meant that our assembleZip task that generates the platform agnostic copy of anathema without a JRE. Once extracted, you run this using the exe launcher, doubleclicking the jar file, or by using the shell script. In the first two cases, it defaults to ./repository, which means that the install is effectively self contained to that folder. If the user wanted to upgrade, they'd probably download another copy, and extract to another folder. Then they would either move their old repository over to the new folder, or copy the new folder's files over into the old folder, replacing old jars.

The NSIS installer just creates a repository directory in the install location, and grants systemwide read/write/execute access to it. If the directory already exists, nothing happens. The NSIS uninstaller removes the directory. I'm pretty sure the installer will just overwrite any old install files during the install process.

Quick Benchmarks

Before/After numbers for the current trunk vs my branch.

Before

./gradlew clean
initial full run of ./gradlew assembleZip - 22.520 secs
./gradlew clean
second full run of ./gradlew assembleZip - 21.657 secs

first repeat run of ./gradlew assembleZip - 12.422 secs
second repeat run of ./gradlew assembleZip - 11.653 secs

average launch time of Anathema (opening jar file, stopwatch, 5 trials) - 7.02 seconds

After

./gradlew clean
initial full run of ./gradlew assembleZip - 28.938 secs
./gradlew clean
second full run of ./gradlew assembleZip - 28.605 secs

first repeat run of ./gradlew assembleZip - 18.532 secs
second repeat run of ./gradlew assembleZip - 18.190 secs

average launch time of Anathema (opening jar file, stopwatch, 5 trials) - 6.20 seconds

I think I know where the bottleneck is in my building, it has to do with the way I'm generating the explicit classpath. I'll clean it up later. The anathema launch times were slightly faster overall.

update:
Times are updated with the proper gradle settings, in particular giving it a normal amount of memory, and enabling daemon builds. This massively speeds up all command line building.

@curttasker
Copy link
Member Author

sigh. Apparently several months ago @UrsKR upgraded Gradle af0f12e, but accidentally removed my command line build speed optimizations to gradle.

I've revised the above benchmarks based on this.

@UrsKR
Copy link
Member

UrsKR commented Jul 24, 2013

Self-contained

Okay, that's what I thought.
Moving the repo around appears to be an issue for the less technically minded, and a working auto-update would take care of this.

Auto-Update

I did a quick survey on G+, and apparently, those guys actually use it.

Benchmark

Actually, I removed the (memory) setting on purpose. I thought it was added by Sean because the build broke on his system, and since the gradle guys promised that they had optimized their memory behaviour, I wanted to give it a try. If there was anything else in there, it was an accident.
Sorry about that.

Your numbers, do they mean that the build takes longer, but only due to the bottleneck you mention?
The advantage at launch, while not impressive, is nice.

Overall impression

I have a growing feeling that dropping the updater for the moment isn't worse than dropping any of the other features we kicked during our cleanup run, and that the advantages win out in the short and in the long run.
I'd like to ask Sandra, though, when I speak to her this evening, to see how important a replacement would be.
I believe to remember the updater was important to her.

@curttasker
Copy link
Member Author

Self-contained

To be clear, nobody is required to move the repository around when manually updating unless they explicitly choose the Zip distribution and execute it by "java -jar Anathema.jar" or by double clicking the jar file. And even this is alleviated by setting a new location for the repository in Preferences.

Repository Location

I have two opposing thoughts on this:

  1. Move the repository universally to ~/.anathema/repository, so we as developers can stop keeping track of where it exists on what platform.
  2. Keep the OS specific locations for Mac/Linux, and update the Windows location to where data of this type should be stored, ~/AppData/Roaming/Anathema

I expect I'll develop further thoughts on the repository location once I finish collecting my thoughts on the repository file formats.

Auto-Update

Why doesn't it surprise me that the Venn Diagram of people using G+ and people using our auto-update has a natural intersection? :)

We should probably talk @davidhfe into making us an in-depth user survey at some point. There's lots of little bits of information that'd help us make decisions. For this question, I'd like to know how often people are updating anathema, and how often they're running it, how often they check for updates, how frequently they update after finding out an update is available, and why they update (features, stability, boredom).

Benchmark

The memory settings shave 10-20 seconds off assembleZip, for example, definitely worth keeping. The default memory they allocate for gradle is criminally small.

Yes, the numbers are higher due to the bottleneck; I suspect its in the code I use to build the dynamic classpath. Basically its iterating over all subprojects, over all depencencies, over all jar files, then creating a giant set of all jar files, throwing out duplicates, stripping out the path info, prepnding "lib/", then creating a giant string out of it all. I'll rework it to something more sane to get the time down. I'll also fork off the trunk again and benchmark my copyDependencies code with that, to see if its any faster/slower than the current method.

Overall

I'm on the fence myself, but mostly because on one hand I hate to toss working code, but on the other, I worry that its going to need to get thrown out anyways in the future, due to packaging changes, app stores, code signing, etc.

@UrsKR
Copy link
Member

UrsKR commented Jul 24, 2013

So, maybe it's just not the moment to make the decision? If somebody wants to change the bundling or go for an appstore or whatever, we can decide really fast now, since we have the facts now.
As long as the updater and bootloader don't hinder us, we don't have to drop them - and I don't think your pull request is critically hard to reproduce at any later time.

@UrsKR
Copy link
Member

UrsKR commented Aug 25, 2013

"As long as the bootloader doesn't hinder us..."

It does now. Apparently, the JavaFX Application class expects its subclass to be on the system classpath.

Note: With this fact known, I think it's best to do as we suggested above: Drop updating for now, convert to a standard classpath/launch system (reaping the benefits as we go) and re-implement auto-updates at a later time.
There are more important features that I dropped along the way.

I've created a Trello card for the problem.

@curttasker
Copy link
Member Author

I think using oracle's javafx packaging tools will alleviate this problem. I'll spend this week getting the build system working with this system.

@UrsKR
Copy link
Member

UrsKR commented Aug 25, 2013

That's great. I'm sure you are aware of the Gradle JavaFX Plugin, in case you need a more gradly syntax.

By 'alleviate', do you mean that it will work with the current mechanism, or that we'll switch over to the standard way of launching and building the classpath? As I said above, I'm fine with either way, I'm just curious.

@UrsKR
Copy link
Member

UrsKR commented Aug 25, 2013

In case the splashscreen code gets in your way, deal with it in whatever way you like. I'll probably re-build that part later using FX's preloader mechanism.

@UrsKR
Copy link
Member

UrsKR commented Aug 25, 2013

I read through the page you linked to, and some related blog you mentioned some months back.
The list of caveats there mentions that all builds have to happen on the target platform.

I am wondering whether there is a way around that by now.

@UrsKR
Copy link
Member

UrsKR commented Aug 26, 2013

@curttasker: Ryan Gray, one of our more active followers on G+, just shared an interesting data point: He updated from 4.3.0 to 5.1.2 just yesterday. So maybe you're right and people don't update as frequently!

@UrsKR
Copy link
Member

UrsKR commented Sep 12, 2013

@curttasker, did you already find time to look at this?

@curttasker
Copy link
Member Author

@UrsKR I've been researching and poking at it a little, but haven't had the solid chunk of time I need to get it ready for roll-out yet. Hopefully early next week, my work schedule's clearing up finally.

@curttasker
Copy link
Member Author

@UrsKR I'm making progress, but its slow going.

I have the javafx gradle plugin building a app file and dmg release on the mac, though I need to tweak our launching and classloading code (similar to what I did in this pull request) to get it launching anathema from this automatically built binary properly. After that, I'll tackle the other platforms.

One minor wrinkle: I'm not entirely sure that you can bundle the JRE using javaFX with Java7, we may have to jump to Java8 early. I'll update more on this as I discover it.

@UrsKR
Copy link
Member

UrsKR commented Sep 23, 2013

That's good to hear.

I am pretty sure bundling the JRE is already possible. If you look at the blog post I linked to above, it's explicitly mentioned. The official documentation gives that impression as well, and is written for JavaFX 2, i.e. the version bundled with Java 7.

Going to Java 8 early would be unfortunate, but not impossible. There are some issues still unresolved in their current builds, and we'd have to work around them.

@UrsKR
Copy link
Member

UrsKR commented Apr 19, 2014

I have made good progress updating Anathema to Java 8 over the past week, and it works flawlessly now.
Did you spent any time on this since last we talked, @curttasker?

If not, I'll try to apply your work to the Java 8-branch, move the launcher to static classpaths and drop the auto-update.

@curttasker
Copy link
Member Author

I haven't spent nearly as much time as I'd have liked; With the delays for
the new Exalted version, I haven't been terribly worried about rushing
things.

Most of my work on the bootloader stuff was stymied by lots of minor issues
getting stuff packaged together properly with JavaFX. The stuff from this
pull request worked fine for that particular snapshot of Anathema, but
porting it to the JavaFX implementation we had 6 months ago was rough
going. If we're 100% Java8, it will be a lot easier to work with.

On Sat, Apr 19, 2014 at 9:50 AM, Urs Reupke notifications@github.comwrote:

I have made good progress updating Anathema to Java 8 over the past week,
and it works flawlessly now.
Did you spent any time on this since last we talked, @curttaskerhttps://github.com/curttasker
?

If not, I'll try to apply your work to the Java 8-branch, move the
launcher to static classpaths and drop the auto-update.


Reply to this email directly or view it on GitHubhttps://github.com//pull/349#issuecomment-40874317
.

@UrsKR
Copy link
Member

UrsKR commented Apr 22, 2014

We are on Java 8 now.
Do you think porting your work to the current codebase is a proper next step?

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

Successfully merging this pull request may close these issues.

None yet

2 participants