Fixes Memory Leak #231

leoarc · 2020-03-10T00:00:15Z

Fixes #228
Before :

After :

All unused tensors inside the runPredict() are being disposed . Performance for multiple predictions has improved a lot. Running multiple predictions dosen't overflow memory now.

leoarc · 2020-03-10T00:02:05Z

@Insiyaa @birm Please have a look at it and suggest necessary changes.

Insiyaa · 2020-03-10T07:02:19Z

apps/model/model.js

        let img2;
+       tf.tidy(()=>{


img2 is hanging after tf.tidy

Insiyaa · 2020-03-10T07:04:30Z

apps/model/model.js

          // normalized.mean().print(true); // Uncomment to check mean value.
          // let min = img2.min();
          // let max = img2.max();
          // let normalized = img2.sub(min).div(max.sub(min));
        } else {
          // Pixel Standardization: scale pixel values to have a zero mean and unit variance.
-
+            tf.tidy(()=> {


Where is normalized returned to

to keep tf.tidy() from destroying the tensor it needs to be returned. normalized is being used afterwards so it doesn't need to be destroyed.

The returned object has to be assigned

I am using tf.keep() instead of return. Does this solve the issue?

It still needs to be assigned, please refer the usage here: https://js.tensorflow.org/api/latest/#keep

Insiyaa · 2020-03-10T07:07:06Z

The mobilenet example might help you with placing these calls. Please let me know if you are facing some issues.

leoarc · 2020-03-10T07:48:28Z

@Insiyaa Tried fixing the issues . Let me know if they are ok .

Insiyaa · 2020-03-10T08:14:51Z

You can enclose into tf.tidy starting from here till the end of the for block. On the predict call, remove await and make it dataSync(). This should help, the same changes are to be done for segment app.

leoarc · 2020-03-10T10:53:42Z

On the predict call, remove await and make it dataSync().

This doesn't work . As given in the documentation here tf.dataSync() is used to download values from a tensor (which is used to store tensor to a js array link ) . I don't know what it is doing here but code runs fine if it is commented. So removing await in the predict call makes it return a promise to values and thus causes errors in the next steps.

leoarc · 2020-03-10T10:57:22Z

@Insiyaa As you mentioned earlier , now I have assigned all the variables returned by tf.tidy().
Please review.

Insiyaa · 2020-03-10T18:29:20Z

Oh yes, that is just a warmup run and we don't really need to get the results, so removing that line would do just good.
What I mean to suggest is that just one tf.tidy call enclosing everything is needed, this line being included or not depends on whether the values are obtained sync (i.e. - let values = model.predict(batched).dataSync(); goes inside the tidy call) or async (let it stay outside). Since the predictions happen patch-wise, I believe it should go about synchronously.

leoarc · 2020-03-10T20:28:05Z

@Insiyaa So i started from the beginning. Now the for loop has only call to tf.tidy() . There is one other call to tf.tidy() here used to dispose the tf.zeros(..) tensor. Please review .

Insiyaa · 2020-03-11T15:13:57Z

apps/model/model.js

@@ -471,10 +473,11 @@ function runPredict(key) {
    self.showProgress("Model loaded...");

    // Warmup the model before using real data.
+    tf.tidy(()=>{
    const warmupResult = model.predict(tf.zeros([1, image_size, image_size, input_channels]));
    warmupResult.dataSync();


I realise the line 478 isn't required to get the values, could you remove it?

Insiyaa · 2020-03-11T15:17:51Z

That seems good, thank you! The same additions would go into segment.js. Also, could you squash the commits into one with a meaningful commit message :)

birm · 2020-03-11T16:09:14Z

If you prefer not to mess with rebasing, we can squash when merging the PR itself.

leoarc · 2020-03-12T13:20:26Z

So i am facing a problem. This line is returning a tensor which I am not being able to dispose . The documentation says tf.browser.toPixels returns a promise of type Uint8ClampedArray and not a tensor . So I am not able to do let x = await tf.toPixels(...) and x.dispose() . And I suppose this function has to be asynchronous .Any ideas ? @Insiyaa

leoarc · 2020-03-14T08:06:55Z

@birm @Insiyaa Should we close this for now ? It still has the issue which I mentioned in the last comment . The issue has been mentioned here and though it is showing solved I think it isn't . Anyways it must be a internal leak . Please review.

Insiyaa · 2020-03-14T15:42:24Z

In my view, all the required additions are done. You're right; there is probably some leak inside tf.browser.toPixels, we can keep track of it. Also, you might want to look into backends if it helps with this issue.
@birm would you like to suggest any changes?

birm

I'm not super picky on formatting now, so this seems fine. Thanks!

Fixes Memory Leak

a7c60a2

birm added the bugfix label Mar 10, 2020

birm self-requested a review March 10, 2020 02:19

Insiyaa reviewed Mar 10, 2020

View reviewed changes

apps/model/model.js Outdated

let img2;

tf.tidy(()=>{

Copy link

Contributor

Insiyaa Mar 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

img2 is hanging after tf.tidy

Insiyaa reviewed Mar 10, 2020

View reviewed changes

leoarc added 2 commits March 10, 2020 12:58

fixed issues

09e1d37

Some changes

46bf8e5

return values assigned

782cd5b

leoarc added 3 commits March 11, 2020 01:15

one tidy call

bf386b4

changes

904545f

fixed-spaces

e6d116b

Insiyaa reviewed Mar 11, 2020

View reviewed changes

updated segment.js

211aa1b

birm approved these changes Mar 14, 2020

View reviewed changes

birm merged commit f047436 into camicroscope:develop Mar 14, 2020

birm mentioned this pull request Mar 15, 2020

Additional Memory being allocated every time Prediction is done #228

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes Memory Leak #231

Fixes Memory Leak #231

leoarc commented Mar 10, 2020 •

edited

Loading

leoarc commented Mar 10, 2020 •

edited

Loading

Insiyaa Mar 10, 2020

Insiyaa Mar 10, 2020

leoarc Mar 10, 2020

Insiyaa Mar 10, 2020

leoarc Mar 10, 2020

Insiyaa Mar 10, 2020

Insiyaa commented Mar 10, 2020 •

edited

Loading

leoarc commented Mar 10, 2020

Insiyaa commented Mar 10, 2020

leoarc commented Mar 10, 2020

leoarc commented Mar 10, 2020

Insiyaa commented Mar 10, 2020

leoarc commented Mar 10, 2020

Insiyaa Mar 11, 2020 •

edited

Loading

Insiyaa commented Mar 11, 2020

birm commented Mar 11, 2020

leoarc commented Mar 12, 2020 •

edited

Loading

leoarc commented Mar 14, 2020

Insiyaa commented Mar 14, 2020

birm left a comment

Fixes Memory Leak #231

Fixes Memory Leak #231

Conversation

leoarc commented Mar 10, 2020 • edited Loading

leoarc commented Mar 10, 2020 • edited Loading

Insiyaa Mar 10, 2020

Choose a reason for hiding this comment

Insiyaa Mar 10, 2020

Choose a reason for hiding this comment

leoarc Mar 10, 2020

Choose a reason for hiding this comment

Insiyaa Mar 10, 2020

Choose a reason for hiding this comment

leoarc Mar 10, 2020

Choose a reason for hiding this comment

Insiyaa Mar 10, 2020

Choose a reason for hiding this comment

Insiyaa commented Mar 10, 2020 • edited Loading

leoarc commented Mar 10, 2020

Insiyaa commented Mar 10, 2020

leoarc commented Mar 10, 2020

leoarc commented Mar 10, 2020

Insiyaa commented Mar 10, 2020

leoarc commented Mar 10, 2020

Insiyaa Mar 11, 2020 • edited Loading

Choose a reason for hiding this comment

Insiyaa commented Mar 11, 2020

birm commented Mar 11, 2020

leoarc commented Mar 12, 2020 • edited Loading

leoarc commented Mar 14, 2020

Insiyaa commented Mar 14, 2020

birm left a comment

Choose a reason for hiding this comment

leoarc commented Mar 10, 2020 •

edited

Loading

leoarc commented Mar 10, 2020 •

edited

Loading

Insiyaa commented Mar 10, 2020 •

edited

Loading

Insiyaa Mar 11, 2020 •

edited

Loading

leoarc commented Mar 12, 2020 •

edited

Loading