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

Crashing with big images in demo #1122

Closed
jywarren opened this issue Jun 21, 2019 · 27 comments
Closed

Crashing with big images in demo #1122

jywarren opened this issue Jun 21, 2019 · 27 comments
Labels

Comments

@jywarren
Copy link
Member

I loaded http://beta.sequencer.publiclab.org with a large image and after a few minutes, saw:

image

Not sure if this is related to #1093 ?

@Divy123
Copy link
Member

Divy123 commented Jun 21, 2019

Can you please upload the image somewhere so that I can test it out locally?

@jywarren
Copy link
Member Author

IMG_20190615_115846

It wasnt this image but one of this size! From my phone!

@harshkhandeparkar
Copy link
Member

I tried this on my phone. The image takes WAY to long to load. So long that it can be considered as a crash. The browser insists on stopping the script. Trying it on my PC now.

@Divy123
Copy link
Member

Divy123 commented Jun 21, 2019

Same with me.
I tested it out on my main which is not updated with the wasm PR as well, so probably not something to do with wasm.

@jywarren
Copy link
Member Author

Perhaps we can pursue #922 which would give people option to resize as the first step?

#922 also suggests consolidating the 'resize' that's used in the preview generator, so it resizes once, then passes the small image to each preview generator, instead of resizing in each generator, this will likely make a BIG difference. Shall we try this next?

@harshkhandeparkar
Copy link
Member

I tried it on a huge 4K image. It didn't crash as such but the browser insisted on stopping the script several times but only at the start(because of previews perhaps). After loading, it works fine but still takes some time to process.

@Divy123
Copy link
Member

Divy123 commented Jun 21, 2019

So, I think the problem is maybe due to previews.
@aashna27 if you have this in your proposal #922 or should I persue that now.

@harshkhandeparkar
Copy link
Member

That preview proposal is extremely important I feel. On phones for e.g., previews make IS unusable.

@jywarren
Copy link
Member Author

Agreed, all! 🙌

@Divy123
Copy link
Member

Divy123 commented Jun 21, 2019

@aashna27 .

@aashna27
Copy link

Hey, just yea i do have this in my proposal and would pursue this one after opencv.

@Divy123
Copy link
Member

Divy123 commented Jun 21, 2019 via email

@harshkhandeparkar
Copy link
Member

@aashna27 @Divy123 @jywarren have a look at #955

@Divy123
Copy link
Member

Divy123 commented Jun 24, 2019

#1127

@MargaretAN9
Copy link

Some general comments:
-I was hoping to come up with a more comprehensive solution but I thought it might be best to share some preliminary test results---

-This is a really important topic since many users will want to process high resolution data. My general view is that IS needs to work with images at least 10MB / (3280x2464). This would match the Raspberry PI image formats as well as other camera formats.

-I have noticed that the ''page unresponsive" notice that may occur on loading large file sizes (~3MB) generally occurs at the initial 'Select or drag in an image to start/Choose file/Take a Photo' prompt box. So, if you were to load a small file first (or keep the example image) and then use the select a module/import image, it generally works. I assumed this ‘initial load size limit’ has something to do with the ’Take a photo’ or ‘select/drag an image’ options since they seem to be the only difference between different loading options, but, I can’t say for sure.

-Another observation is that different sequences may or may not work with large file sizes. For example, a 40 MB file can be loaded within 5 seconds (just not at the ‘Take a photo’ prompt) and can be easily Inverted (within a few seconds) or flipped. If you were to take the same 40MB image and use the contrast module, you will receive ‘page unresponsive/Aw Snap something went wrong’; the 40MB image with ‘detect edges’ results in a never ending spinning circle with no error response. The three different outcomes (works, unresponsive message, spinning circle) seem to be dependent on the level of processing required but, again, I can say for certain. Failures may not just be about total file size, other restrictions such as #of Horizontal vs Vertical pixels may also limit processing.

-My download IS record is 110MB! At 110MB, the sequences that work for 40MB (invert and flip) provide unresponsive errors. So you can’t really do much with IS and very very large file sizes but its possible to download 100MB files! (270MB attempt yielded unresponsive message so max limit seems to be between 110 and 270MB).

Options for way forward:

-1)The most significant problem is that the initial prompt to load an image has inconsistent performance based on file size. This is unfortunate since it’s the first thing that new users may experience. The first place to look is whether the camera/drag an image option interferes with the load process, the other option is to eliminate the opening sequence all together and import images direct from the drop box menu since this seems to be reliable.

-Another recommendation is to routinely run/test all modules with larger file sizes and with different dimensions. Additional testing needs to be done to determine if other modules besides ‘Select or drag an image” have unresponsive errors for large file sizes. Users will become frustrated if they can successfully load an image and then be unable to process the image. I have loaded some practice 4.7Mb and 12.5MB files on github that can be used for testing https://github.com/MargaretAN9/GSOC-2019/tree/master/IS%20test%20picture%20large%20size .
Github has a 25MB file size limit but a 4OMB file available at:
http://www.effigis.com/wp-content/uploads/2015/02/Airbus_Pleiades_50cm_8bit_RGB_Yogyakarta.jpg
One option may to label ‘slow’ modules as processing intensive so the user can anticipate the extra processing time.

-The last comment is that it will be interesting to see if OpenCV provides faster processing times. If this is the case, it may be possible to rewrite some IS modules with OpenCV functions. OpenCV also provides compression options that may also help in reducing file size. (Of course, we need to get it working first…).

@harshkhandeparkar
Copy link
Member

Hi all. This is becoming a big problem. Big images don't even load in some browser. In some browsers like Firefox, they slow it down to a point when the browser insists on stopping the script as it the script is considered unresponsive.

I am going to conduct a series of experiments like running a profiler, tweaking some code, turning off previews, etc. I will try to narrow down the slow code. Posting the results shortly.
cc @publiclab/is-reviewers @Divy123 @MargaretAN9

@harshkhandeparkar
Copy link
Member

Report

With Previews (stock image)

  • Some function called drainQueue(probably an external lib function) takes quite some time.
  • GC is done thousands of times(Have to optimise)(GC is done about a thousand times at once which takes a lot, lot of time)
  • The img.onload event listeners in two different places singly take quite some time to process.
img.onload = function() {
    var canvas = document.createElement('canvas')
    canvas.width = img.width
    canvas.height = img.height
    var context = canvas.getContext('2d')
    context.drawImage(img, 0, 0)
    var pixels = context.getImageData(0, 0, img.width, img.height)
    cb(null, ndarray(new Uint8Array(pixels.data), [img.width, img.height, 4], [4, 4*img.width, 1], 0))
  }
else if (ref.options.inBrowser) {
      var ext = src.split('.').pop();
      var image = document.createElement('img');
      var canvas = document.createElement('canvas');
      var context = canvas.getContext('2d');
      image.onload = function() {
        canvas.width = image.naturalWidth;
        canvas.height = image.naturalHeight;
        context.drawImage(image, 0, 0);
        datauri = canvas.toDataURL(ext);
        callback(datauri, step);
      };
      image.src = src;
    }
Conclusion: Takes about 5s with stock image (JS function calls take the longest time)

@harshkhandeparkar
Copy link
Member

Without Previews (Stock Image)

  • Very Little GC
  • No single function call is run for too long
  • About 1s to process

Bigger External Image (No Previews)

  • Still very little GC
  • Still no single function call is run for too long
  • Still 1s to process

Above 4K size image (No Previews)

  • Some weird mousemove event took 1.5s(Might be related to @aashna27's pixel data display feature.
  • Still little GC
  • 2.5s in total(including mousemove)

@harshkhandeparkar
Copy link
Member

Conclusion

  • There might be some unoptimized core code.
  • GC is a problem.
  • Previews cause the most GC
  • MouseMove was weird.

@harshkhandeparkar
Copy link
Member

cc @jywarren @publiclab/is-reviewers @tech4GT @Divy123 @aashna27 @MargaretAN9

@aashna27
Copy link

Without Previews (Stock Image)

* Very Little GC

* No single function call is run for too long

* About 1s to process

Bigger External Image (No Previews)

* Still very little GC

* Still no single function call is run for too long

* Still 1s to process

Above 4K size image (No Previews)

* Some weird `mousemove` event took 1.5s(Might be related to @aashna27's pixel data display feature.

* Still little GC

* 2.5s in total(including mousemove)

May I ask what is GC? 🙈

@harshkhandeparkar
Copy link
Member

@harshkhandeparkar
Copy link
Member

We can reduce GC by not setting too many variables or by setting them to null so that there is no memory leak.

@jywarren
Copy link
Member Author

jywarren commented Jul 9, 2019

Could we - thinking - could we use one sequencer instance for resizing, and just create new ones to do each preview with the output of the first resizing-sequencer?

@aashna27
Copy link

Can we close this now?

@jywarren
Copy link
Member Author

Yes, I think so! Thanks!!!

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

No branches or pull requests

5 participants