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

Make BufferedImageFactory.fromNDArray synchronous #1339

Merged
merged 1 commit into from
Nov 2, 2021

Conversation

zachgk
Copy link
Contributor

@zachgk zachgk commented Nov 2, 2021

I made this PR based on this comment by @steinhae in #1278.

I also ran a quick experiment to verify the findings:

try (NDManager manager = NDManager.newBaseManager()) {
  NDArray array = manager.randomInteger(0, 255, new Shape(3, 400, 400), DataType.INT32).toType(DataType.UINT8, true);
  ImageFactory factory = ImageFactory.getInstance();
  for (int i=0; i<5000; i++) {
      factory.fromNDArray(array);
  }
}

Found that this change would reduce the time to run this test from about 62 seconds to about 22 seconds.

Found that it gives some performance improvement.
@zachgk zachgk requested a review from frankfliu November 2, 2021 20:48
@codecov-commenter
Copy link

codecov-commenter commented Nov 2, 2021

Codecov Report

Merging #1339 (03cdad5) into master (bb5073f) will decrease coverage by 0.53%.
The diff coverage is 54.92%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1339      +/-   ##
============================================
- Coverage     72.08%   71.55%   -0.54%     
- Complexity     5126     5145      +19     
============================================
  Files           473      478       +5     
  Lines         21970    22295     +325     
  Branches       2351     2394      +43     
============================================
+ Hits          15838    15954     +116     
- Misses         4925     5122     +197     
- Partials       1207     1219      +12     
Impacted Files Coverage Δ
.../java/ai/djl/modality/cv/BufferedImageFactory.java 72.68% <ø> (+0.23%) ⬆️
...i/djl/modality/cv/translator/BigGANTranslator.java 21.42% <ø> (-5.24%) ⬇️
...odality/cv/translator/BigGANTranslatorFactory.java 33.33% <0.00%> (+8.33%) ⬆️
...nslator/InstanceSegmentationTranslatorFactory.java 14.28% <0.00%> (-3.90%) ⬇️
.../modality/cv/translator/YoloTranslatorFactory.java 8.33% <0.00%> (-1.67%) ⬇️
...i/djl/modality/cv/translator/YoloV5Translator.java 5.69% <0.00%> (ø)
...odality/cv/translator/YoloV5TranslatorFactory.java 8.33% <0.00%> (-1.67%) ⬇️
...n/java/ai/djl/modality/nlp/bert/BertTokenizer.java 92.68% <0.00%> (-2.32%) ⬇️
...modality/nlp/embedding/TrainableWordEmbedding.java 45.00% <0.00%> (-3.34%) ⬇️
...pi/src/main/java/ai/djl/ndarray/BytesSupplier.java 54.54% <0.00%> (-12.13%) ⬇️
... and 92 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa7ff1d...03cdad5. Read the comment docs.

@zachgk zachgk merged commit 518cadc into deepjavalibrary:master Nov 2, 2021
@zachgk zachgk deleted the fromNDArray branch November 2, 2021 21:19
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.

3 participants