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

In Boosting Assembler wrapping each estimator into a subroutine causes a performance degradation #152

Closed
izeigerman opened this issue Jan 21, 2020 · 6 comments
Assignees

Comments

@izeigerman
Copy link
Member

I've recalled the real motivation behind not wrapping every individual estimator into its own subroutine - generation of many nested function calls leads to a performance degradation in Java. The observed difference reaches 4x for larger models (eg. XGBoost with 1000 estimators). The basic test I created (sorry about Scala):

@ import com.github.m2cgen.ModelOld
import com.github.m2cgen.ModelOld

@ import com.github.m2cgen.ModelNew
import com.github.m2cgen.ModelNew

@ def nextRandomData(): Array[Double] = (0 until 4).map(_ => Random.nextDouble).toArray
defined function nextRandomData

@ def testScore: Unit = {
    val start = System.currentTimeMillis()
    (0 until 100000).foreach(_ => <ModelNew|ModelOld>.score(nextRandomData))
    println("Runtime: " + (System.currentTimeMillis() - start).toString)
  }

Results for ModelOld:

@ testScore
Runtime: 2973

For ModelNew:

@ testScore
Runtime: 10747

The test model has been trained using the sklearn.datasets.load_iris() dataset. Classifier has been created as following:

model = XGBClassifier(n_estimators=1000)

In the attached archive I included the following:

  1. ModelNew.java - java code generated with the most recent master.
  2. ModelOld.java - java code generated with the release 0.5.0 version.
  3. Models.jar - the jar containing both compiled sources.
  4. xgboost_model2 - the trained estimator in Pickle format.

CC: @StrikerRUS FYI

@StrikerRUS
Copy link
Member

StrikerRUS commented Jan 22, 2020

x4 slowdown is very awful! However, Java slowdown due to wrapping estimators into subroutine should be less panful compared to R slowdown due to not wrapping, because slow Java code doesn't exceed Travis limit while R code does this.
Do you have any idea for possible compromise yet?

@izeigerman
Copy link
Member Author

izeigerman commented Jan 22, 2020

Solving this issue on assembler's end rather than in interpreters certainly wasn't the best idea ever. So far I don't have any great ideas, but I'm sure I don't like penalizing one language in favor of the other.

UPD: We should also keep in mind that JVM usage is/will be potentially more widespread and have stricter execution time constraints.

UPD2: I actually do have one idea which I'm going to explore this upcoming weekend (or hopefully sooner).

@izeigerman izeigerman self-assigned this Jan 22, 2020
@izeigerman
Copy link
Member Author

Turns out I've accidentally generated ModelNew.java for a wrong model. After rerunning my tests the numbers look a lot more reasonable for the current master:

@ testScore
Runtime: 3526

The difference is still significant but not nearly as dramatic. Attaching the updated archive
java_boosting_test.tar.gz

@StrikerRUS
Copy link
Member

... but I'm sure I don't like penalizing one language in favor of the other.

I absolutely agree with you! I think we should try to support all our languages equally.

UPD: We should also keep in mind that JVM usage is/will be potentially more widespread and have stricter execution time constraints.

Sorry, I have no any experience in Java 🙁 .

UPD2: I actually do have one idea which I'm going to explore this upcoming weekend (or hopefully sooner).

Sounds promising!

The difference is still significant but not nearly as dramatic.

Still this slowdown is not good thing.

@StrikerRUS
Copy link
Member

Can we close this?

@izeigerman
Copy link
Member Author

Yes, absolutely. Thank you!

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

No branches or pull requests

2 participants