[win32] ImageDataProvider getBounds performance #2564
Conversation
This commit improves the performance of Image#getBounds if an ImageDataProvider is used and no matching handle exists yet. This only affects the windows implementation.
|
For the presented scenario the change definitely improves performance. And it of course makes sense, because when only the bounds for a zoom are of interest, there is no need to create the right image data (via handle creation) that are properly scaled. It is sufficient to just find any representative image data from which we can derive the correct bounds. In my understanding, the proposed change is correct w.r.t. that regard. I only have one concern: I would expect that when retreiving the bounds for a specific zoom, the consumer will likely also request image data or even a handle for that zoom later on. And in that scenario, the change leads to a performance drawback. Take the following snippet (derived from the on in the PR description): public static void main(String[] args) {
Display display = new Display();
ImageDataProvider idp = new ImageDataProvider() {
@Override
public ImageData getImageData(int zoom) {
ImageData data = new ImageData(2000, 2000, 32, new PaletteData(0, 0, 0));
for (int x = 0; x < 2000; x++) {
for (int y = 0; y < 2000; y++) {
data.setPixel(x, y, 1);
}
}
return data;
}
};
long start = System.currentTimeMillis();
for (int i = 0; i < 100; i++) {
Image image = new Image(display, idp);
image.getBounds();
image.getImageData();
}
System.out.println("Duration ImageDataProvider " + (System.currentTimeMillis() - start) + " ms size: " + new Image(display, idp).getBounds());
}On average, I get the following results: One option to improve this might be to cache the image data that were just loaded (but not yet fully "processed", i.e., wrapped into a handle and scaled). I.e., one could add another map to the private ImageHandle initializeHandleFromSource(int zoom) {
ElementAtZoom<ImageData> imageDataAtZoom;
if (loadedImageData.containsKey(zoom)) {
imageDataAtZoom = loadedImageData.get(zoom);
loadedImageData.remove(zoom);
} else {
imageDataAtZoom = loadImageData(zoom);
}
ImageData imageData = DPIUtil.scaleImageData (device, imageDataAtZoom.element(), zoom, imageDataAtZoom.zoom());
imageData = adaptImageDataIfDisabledOrGray(imageData);
return init(imageData, zoom);
}
protected abstract ElementAtZoom<ImageData> loadImageData(int zoom);
@Override
protected Rectangle getBounds(int zoom) {
if (cachedImageData.containsKey(zoom)) {
ImageData imageData = cachedImageData.get(zoom);
return new Rectangle(0, 0, imageData.width, imageData.height);
}
ElementAtZoom<ImageData> imageDataAtZoom;
if (loadedImageData.containsKey(zoom)) {
imageDataAtZoom = loadedImageData.get(zoom);
} else {
imageDataAtZoom = loadImageData(zoom);
loadedImageData.put(zoom, imageDataAtZoom);
}
ImageData imageData = imageDataAtZoom.element();
Rectangle currentSize = new Rectangle(0, 0, imageData.width, imageData.height);
return Win32DPIUtils.scaleBounds(currentSize, zoom, imageDataAtZoom.zoom());
} |
|
After some discussion, due to different scenarios we decided to close this PR for now as it would introduce some drawbacks |
akoch-yatta
deleted the
win32-fix-ImageDataProvider-getBounds-performance
branch
2 participants
This PR improves the performance of
Image#getBoundsif anImageDataProvideris used and no matching handle exists yet. This only affects the windows implementation.Reason for the worse performance is that currently multiple
ImageDatainstances plus an image handle instance is created and cleaned up for the operation, although one call toImageDataProvider#getImageDatawould be sufficient.How to test
Add an big image (at least 1000x1000 pixels) named "bigimage.png" so it can be found by the snippet and execute the snippet below with and without this PR und you'll see something similar to
Without PR:
Duration ImageDataProvider 3216 ms size: Rectangle {0, 0, 2000, 2000}
Duration ImageFileNameProvider 1668 ms size: Rectangle {0, 0, 2000, 2000}
With PR:
Duration ImageDataProvider 369 ms size: Rectangle {0, 0, 2000, 2000}
Duration ImageFileNameProvider 1547 ms size: Rectangle {0, 0, 2000, 2000}
For
ImageDataProviderperformance is drastically improved, forImageFileNameProviderit stays similar. But I think adapting the existing common implementation forgetBoundsmakes more sense as long is it does not have a negative effect.