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

Preload memory cache from disk cache #978

Closed
mtst99 opened this issue Feb 11, 2016 · 34 comments
Closed

Preload memory cache from disk cache #978

mtst99 opened this issue Feb 11, 2016 · 34 comments
Labels

Comments

@mtst99
Copy link

mtst99 commented Feb 11, 2016

It is interesting to see how some applications like A+Gallery or QuickPic show images loaded into the disk cache very fast and smooth.
Imagine I have 1000 images.
They are shown inside a ListView or RecyclerView.
About 20 to 50 images gonna appear on the screen.
On scroll I get to the images that are not inside memory cache anymore because memory cache is now filled with scrolled images.
So memory cache is limited and it could be customized but carefully which is not my concern now.
What I noticed about A+Gallery and QuickPic is that they show images like they are available inside memory cache which is impossible by the size of provided memory cache that these days' devices provide for each application.
So It left me with this idea that these apps loading memory cache from disk cache as soon as ListView or RecyclerView or GridView gets close to the end of the available bitmaps inside memory cache.
Glide loads new images from disk cache if they are not in memory cache, which will happen if users scroll on many images in one direction.
If explained feature is already implemented into the Glide, please help me to know how to load Glide's memory cache from its disk cache to prevent fast scrolling show empty items or placeholders.
And if Glide doesn't have such a feature yet I would like to know your idea about requested feature is it doable or I'm wrong about mentioned apps. Thank you

@TWiStErRob
Copy link
Collaborator

@mtst99
Copy link
Author

mtst99 commented Feb 11, 2016

You're doing a great job with this beautiful library. Thanks for the links.

@TWiStErRob
Copy link
Collaborator

Credit goes to @sjudd! Feel free to ask if you're stuck.

@mtst99
Copy link
Author

mtst99 commented Feb 13, 2016

Would you provide me a simpler example? Imagine I have a list of strings that path of images are in this list and on adapter of ListView i'm using glide to load images. (No thumbnails)

@TWiStErRob
Copy link
Collaborator

Here's a fragment that has a list. The adapter is filled with data by generating some random-colored images. It gets a request from "the outside"; this is not necessary, but good practice, because http://stackoverflow.com/a/32887693/253468.

public class TestFragment extends android.support.v4.app.ListFragment {
    private static final String URL_TEMPLATE = "http://placehold.it/300x200/%06x/000000&text=%d";
    @Override public void onViewCreated(View view, @Nullable Bundle savedInstanceState) {
        super.onViewCreated(view, savedInstanceState);
        List<String> images = new ArrayList<>();
        for (int i = 1; i < 100; ++i) {
            images.add(String.format(Locale.ROOT, URL_TEMPLATE, i * 0x5f3759df % 0x1000000, i));
        }
        DrawableRequestBuilder<String> request = Glide
                .with(this)
                .fromString()
                .fitCenter() // must be explicit, otherwise there's a conflict between
                // into(ImageView) and into(Target) which may lead to cache misses
                .listener(new LoggingListener<String, GlideDrawable>())
        ;

        PreloadingAdapter adapter = new PreloadingAdapter(request, images);
        setListAdapter(adapter);
        getListView().setOnScrollListener(adapter.preload(3));
    }
}

The listener is not necessary, but it's a good way to see what's being loaded, you can find an implementation and explanation on the wiki.

The following is a fully functioning adapter, you can see that the only a few extra things are needed to upgrade a normal adapter to a preloading one.

class PreloadingAdapter extends BaseAdapter implements PreloadModelProvider<String> {
    private final GenericRequestBuilder<String, ?, ?, ?> request;
    private final ViewPreloadSizeProvider<String> sizeProvider = new ViewPreloadSizeProvider<>();
    private final List<String> images;

    public PreloadingAdapter(GenericRequestBuilder<String, ?, ?, ?> request, List<String> images) {
        this.request = request;
        this.images = images;
    }

    /// adapter methods
    @Override public int getCount() {
        return images.size();
    }
    @Override public String getItem(int position) {
        return images.get(position);
    }
    @Override public long getItemId(int position) {
        return images.get(position).hashCode();
    }
    @Override public View getView(int position, View convertView, ViewGroup parent) {
        ImageView imageView = (ImageView)convertView;
        if (imageView == null) {
            imageView = new ImageView(parent.getContext());
            imageView.setLayoutParams(new AbsListView.LayoutParams(LayoutParams.MATCH_PARENT, 400));
            sizeProvider.setView(imageView); // safe to call multiple times
        }
        request.load(getItem(position)).into(imageView);
        return imageView;
    }

    /// preload methods
    @Override public List<String> getPreloadItems(int position) {
        return Collections.singletonList(getItem(position));
    }
    @Override public GenericRequestBuilder getPreloadRequestBuilder(String item) {
        return request.load(item);
    }
    public OnScrollListener preload(int maxPreload) {
        return new ListPreloader<>(this, sizeProvider, maxPreload);
    }
}

To adapt this to a RecyclerView you should be able to just wrap it in RecyclerToListViewScrollListener.

@mtst99
Copy link
Author

mtst99 commented Feb 13, 2016

Thanks a lot. Your links were great.
It may sound silly but I still have problem
Now I want to replace String with my object (ParentObject) which is a parentObject that contains 3 childObject.

parentObject --> row of list
childObject --> contains properties of an image
Each childObject has a String property of path of image.

Normaly I load images with Glide like this >>
for each item of adapter I have:

Glide.with(context).load(parentObject.childObject1.stringMediaDataPath).asBitmap().dontAnimate().centerCrop().into(viewHolderListViewGallery.imageView1);
Glide.with(context).load(parentObject.childObject2.stringMediaDataPath).asBitmap().dontAnimate().centerCrop().into(viewHolderListViewGallery.imageView2);
Glide.with(context).load(parentObject.childObject3.stringMediaDataPath).asBitmap().dontAnimate().centerCrop().into(viewHolderListViewGallery.imageView3);

but now with your guidance I must load images with DrawableRequestBuilder like this:

drawableRequestBuilder.load(parentObject.childObject1.imagePath).asBitmap().dontAnimate().centerCrop().into(viewHolderListViewGallery.imageView1);
drawableRequestBuilder.load(parentObject.childObject2.imagePath).asBitmap().dontAnimate().centerCrop().into(viewHolderListViewGallery.imageView2);
drawableRequestBuilder.load(parentObject.childObject3.imagePath).asBitmap().dontAnimate().centerCrop().into(viewHolderListViewGallery.imageView3);

problem is : now load gets ParentObject as type and i'm sending string. I don't know how should I approach this to load path string from object list with drawableRequestBuilder load.

@TWiStErRob
Copy link
Collaborator

I don't exactly see any problem with the code you pointed out. I suggest you move all those repeated arguments to the initialization of drawableRequestBuilder (you can attach more methods even if you receive it from the outside as a constructor argument), see the .fromString() trick above. Warning: after a call to asBitmap() it'll become BitmapRequestBuilder.

To make the preloading work you just need to return the list of those 3 images from getPreloadItems:

ParentObject parent = getItem(position));
return Arrays.asList(parent.child1.imagePath, parent.child2.imagePath, parent.child3.imagePath);

The preloader and the adapter don't have to have the same generic types.

If you register a model loader for ChildObject (see BaseGlideUrlLoader and ignore the bucketing), you can even call .load(parent.child1) and just return the "list of children", not the "urls of the list of children". Note that you can't do the same for ParentObject because there must be a separate load for each view.

If this doesn't help, please rephase your question.

@mtst99
Copy link
Author

mtst99 commented Feb 13, 2016

This is my adapter inside my MainActivity before implementing PreloadModelProvider

public class BaseAdapterGallery3Item extends BaseAdapter {

    Context context;
    LayoutInflater layoutInflater;

    BaseAdapterGallery3Item(Context context) {
        this.context = context;
    }

    @Override
    public int getItemViewType(int position) {
        return mediaStoreFileRow3List.get(position).mediaStoreFile1.intMediaStoreFileViewType;
    }

    @Override
    public int getViewTypeCount() {
        return 10;
    }

    @Override
    public int getCount() {
        return mediaStoreFileRow3List.size();
    }

    @Override
    public Object getItem(int position) {
        return mediaStoreFileRow3List.get(position);
    }

    @Override
    public long getItemId(int position) {
        return position;
    }

    @Override
    public View getView(int position, View convertView, ViewGroup parent) {
        MediaStoreFileRow3 mediaStoreFileRow3 = mediaStoreFileRow3List.get(position);
        ViewHolderListViewGallery holder;

        if (convertView == null) {
            layoutInflater = (LayoutInflater) context.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
            if (getItemViewType(position) == INT_VIEW_TYPE_PHOTO) {
                convertView = layoutInflater.inflate(R.layout.item_gallery_3, parent, false);
            } else {
                convertView = layoutInflater.inflate(R.layout.item_gallery_3_horizontal, parent, false);
            }

            holder = new ViewHolderListViewGallery(convertView);
            convertView.setTag(holder);
        } else {
            holder = (ViewHolderListViewGallery) convertView.getTag();
        }

        if (mediaStoreFileRow3.mediaStoreFile1.intMediaStoreFileViewType == INT_VIEW_TYPE_PHOTO) {
            holder.imageView1.setVisibility(View.VISIBLE);
            holder.autoResizeTextView1.setVisibility(View.GONE);
            Glide.with(context).load(mediaStoreFileRow3.mediaStoreFile1.stringMediaDataPath).asBitmap().dontAnimate().centerCrop().into(holder.imageView1);
        } else {
            holder.imageView1.setVisibility(View.GONE);
            holder.autoResizeTextView1.setVisibility(mediaStoreFileRow3.mediaStoreFile1.intVisibility);
            holder.autoResizeTextView1.setBackgroundColor(mediaStoreFileRow3.mediaStoreFile1.intBackgroundColor);
            holder.autoResizeTextView1.setText(mediaStoreFileRow3.mediaStoreFile1.stringDayMonthYearDateTaken);
        }

        if (mediaStoreFileRow3.mediaStoreFile2.intMediaStoreFileViewType == INT_VIEW_TYPE_PHOTO) {
            holder.imageView2.setVisibility(View.VISIBLE);
            holder.autoResizeTextView2.setVisibility(View.GONE);
            Glide.with(context).load(mediaStoreFileRow3.mediaStoreFile2.stringMediaDataPath).asBitmap().dontAnimate().centerCrop().into(holder.imageView2);
        } else {
            holder.imageView2.setVisibility(View.GONE);
            holder.autoResizeTextView2.setVisibility(mediaStoreFileRow3.mediaStoreFile2.intVisibility);
            holder.autoResizeTextView2.setBackgroundColor(mediaStoreFileRow3.mediaStoreFile2.intBackgroundColor);
            holder.autoResizeTextView2.setText(mediaStoreFileRow3.mediaStoreFile2.stringDayMonthYearDateTaken);
        }

        if (mediaStoreFileRow3.mediaStoreFile3.intMediaStoreFileViewType == INT_VIEW_TYPE_PHOTO) {
            holder.imageView3.setVisibility(View.VISIBLE);
            holder.autoResizeTextView3.setVisibility(View.GONE);
            Glide.with(context).load(mediaStoreFileRow3.mediaStoreFile3.stringMediaDataPath).asBitmap().dontAnimate().centerCrop().into(holder.imageView3);
        } else {
            holder.imageView3.setVisibility(View.GONE);
            holder.autoResizeTextView3.setVisibility(mediaStoreFileRow3.mediaStoreFile3.intVisibility);
            holder.autoResizeTextView3.setBackgroundColor(mediaStoreFileRow3.mediaStoreFile3.intBackgroundColor);
            holder.autoResizeTextView3.setText(mediaStoreFileRow3.mediaStoreFile3.stringDayMonthYearDateTaken);
        }
        return convertView;
    }
}

Now I'm trying to add preloader

inside fields:

DrawableRequestBuilder<String> drawableRequestBuilder;

inside onCreate:

drawableRequestBuilder = Glide
            .with(this)
            .fromString()
            .fitCenter();

listViewGallery.setAdapter(baseAdapterGallery3Item);
listViewGallery.setOnScrollListener(baseAdapterGallery3Item.preload(10));

and this is my new adapter:

public class BaseAdapterGallery3Item extends BaseAdapter implements ListPreloader.PreloadModelProvider<String> {

    Context context;
    LayoutInflater layoutInflater;
    private final ViewPreloadSizeProvider<String> viewPreloadSizeProvider = new ViewPreloadSizeProvider<>();

    BaseAdapterGallery3Item(Context context) {
        this.context = context;
    }

    @Override
    public int getItemViewType(int position) {
        return mediaStoreFileRow3List.get(position).mediaStoreFile1.intMediaStoreFileViewType;
    }

    @Override
    public int getViewTypeCount() {
        return 10;
    }

    @Override
    public int getCount() {
        return mediaStoreFileRow3List.size();
    }

    @Override
    public Object getItem(int position) {
        return mediaStoreFileRow3List.get(position);
    }

    @Override
    public long getItemId(int position) {
        return position;
    }

    @Override
    public View getView(int position, View convertView, ViewGroup parent) {
        final MediaStoreFileRow3 mediaStoreFileRow3 = mediaStoreFileRow3List.get(position);

        ViewHolderListViewGallery viewHolderListViewGallery;

        if (convertView == null) {
            layoutInflater = (LayoutInflater) context.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
            if (getItemViewType(position) == INT_VIEW_TYPE_PHOTO) {
                convertView = layoutInflater.inflate(R.layout.item_gallery_3, parent, false);
            } else {
                convertView = layoutInflater.inflate(R.layout.item_gallery_3_horizontal, parent, false);
            }
 //                convertView.getLayoutParams().height = convertDpToPx(context, 30);
            viewHolderListViewGallery = new ViewHolderListViewGallery(convertView);
            convertView.setTag(viewHolderListViewGallery);
            viewPreloadSizeProvider.setView(convertView.findViewById(R.id.image_view_1));
        } else {
            viewHolderListViewGallery = (ViewHolderListViewGallery) convertView.getTag();
        }

        if (mediaStoreFileRow3.mediaStoreFile1.intMediaStoreFileViewType == INT_VIEW_TYPE_PHOTO) {
            viewHolderListViewGallery.imageView1.setVisibility(View.VISIBLE);
            viewHolderListViewGallery.autoResizeTextView1.setVisibility(View.GONE);
 //                Glide.with(context).load(mediaStoreFileRow3.mediaStoreFile1.stringMediaDataPath).asBitmap().dontAnimate().centerCrop().into(viewHolderListViewGallery.imageView1);
            drawableRequestBuilder.load(mediaStoreFileRow3.mediaStoreFile1.stringMediaDataPath).dontAnimate().centerCrop().into(viewHolderListViewGallery.imageView1);

        } else {
            viewHolderListViewGallery.imageView1.setVisibility(View.GONE);
            viewHolderListViewGallery.autoResizeTextView1.setVisibility(mediaStoreFileRow3.mediaStoreFile1.intVisibility);
            viewHolderListViewGallery.autoResizeTextView1.setBackgroundColor(mediaStoreFileRow3.mediaStoreFile1.intBackgroundColor);
            viewHolderListViewGallery.autoResizeTextView1.setText(mediaStoreFileRow3.mediaStoreFile1.stringDayMonthYearDateTaken);
        }

        if (mediaStoreFileRow3.mediaStoreFile2.intMediaStoreFileViewType == INT_VIEW_TYPE_PHOTO) {
            viewHolderListViewGallery.imageView2.setVisibility(View.VISIBLE);
            viewHolderListViewGallery.autoResizeTextView2.setVisibility(View.GONE);
 //                Glide.with(context).load(mediaStoreFileRow3.mediaStoreFile2.stringMediaDataPath).asBitmap().dontAnimate().centerCrop().into(viewHolderListViewGallery.imageView2);
            drawableRequestBuilder.load(mediaStoreFileRow3.mediaStoreFile2.stringMediaDataPath).dontAnimate().centerCrop().into(viewHolderListViewGallery.imageView2);
        } else {
            viewHolderListViewGallery.imageView2.setVisibility(View.GONE);
            viewHolderListViewGallery.autoResizeTextView2.setVisibility(mediaStoreFileRow3.mediaStoreFile2.intVisibility);
            viewHolderListViewGallery.autoResizeTextView2.setBackgroundColor(mediaStoreFileRow3.mediaStoreFile2.intBackgroundColor);
            viewHolderListViewGallery.autoResizeTextView2.setText(mediaStoreFileRow3.mediaStoreFile2.stringDayMonthYearDateTaken);
        }

        if (mediaStoreFileRow3.mediaStoreFile3.intMediaStoreFileViewType == INT_VIEW_TYPE_PHOTO) {
            viewHolderListViewGallery.imageView3.setVisibility(View.VISIBLE);
            viewHolderListViewGallery.autoResizeTextView3.setVisibility(View.GONE);
 //                Glide.with(context).load(mediaStoreFileRow3.mediaStoreFile3.stringMediaDataPath).asBitmap().dontAnimate().centerCrop().into(viewHolderListViewGallery.imageView3);
            drawableRequestBuilder.load(mediaStoreFileRow3.mediaStoreFile3.stringMediaDataPath).dontAnimate().centerCrop().into(viewHolderListViewGallery.imageView3);
        } else {
            viewHolderListViewGallery.imageView3.setVisibility(View.GONE);
            viewHolderListViewGallery.autoResizeTextView3.setVisibility(mediaStoreFileRow3.mediaStoreFile3.intVisibility);
            viewHolderListViewGallery.autoResizeTextView3.setBackgroundColor(mediaStoreFileRow3.mediaStoreFile3.intBackgroundColor);
            viewHolderListViewGallery.autoResizeTextView3.setText(mediaStoreFileRow3.mediaStoreFile3.stringDayMonthYearDateTaken);
        }
        return convertView;
    }

    @Override
    public List<String> getPreloadItems(int position) {
        return Arrays.asList(mediaStoreFileRow3List.get(position).mediaStoreFile1.stringMediaDataPath, mediaStoreFileRow3List.get(position).mediaStoreFile2.stringMediaDataPath, mediaStoreFileRow3List.get(position).mediaStoreFile3.stringMediaDataPath);
    }

    @Override
    public GenericRequestBuilder getPreloadRequestBuilder(String item) {

        return drawableRequestBuilder.load(item);
    }

    public AbsListView.OnScrollListener preload(int maxPreload) {
        return new ListPreloader<>(this, viewPreloadSizeProvider, maxPreload);
    }
}

Now you can see in provided video only some of the images are preloaded into cache memory when I scroll fast. I don't know where is my mistake on implementing preloader. I've set preload number to 50 or 100 and no result out of it and app crashed by number around 200. On number 10 at least I saw some of images preloaded. lower numbers not any significant result.

I'm using "AZ screen recorder" and while recording this video number of preloaded images differs only in two or three but it explains some expensive behaviour is behind this latency while mentioned softwares are very smooth and fast even on super fast scrolling.

https://youtu.be/LbvBTDgvCNQ

@TWiStErRob
Copy link
Collaborator

You seem to be emulating a GridView with this adapter, why not just use one? :)

There are a few potential issues above:

  • you hide the ImageView. This may mess up the preloader because ViewTarget.SizeDeterminer uses the pre-draw-listener to figure out how big the image should be. Pre-draw only happens if the view is visible. You should probably change to FixedPreloadSizeProvider and use .override() on drawableRequestBuilder to force Glide to load the same size.

  • simply hiding the ImageView may also lead to leaks. The ImageView and Glide still hold on the resources resulting in worse performance because Bitmaps cannot be reused and memory may be always full. Add Glide.clear(viewHolderListViewGallery.imageViewX) before each setVisibility(GONE).

  • you have contradicting duplicate code. .fitCenter() at creation time and .centerCrop() at load time. I suggest you move .dontAnimate().centerCrop() to onCreate. This also affects preloader, because it doesn't preload at all! See next.

  • preloadRequestBuilder and real loads don't match up. There's no point in preloading anything if you don't hit the cache. The transformation is definitely different (centerCrop VS fitCenter), that alone is enough to miss the cache. Also size and many other factors (loader, fetcher, decoder, transcoder, ...) all contribute to the cache key. If you want to make a hit you should match up all this. This is why it's important to use the same request builder for preload. To figure out if the cache hits, enable detailed logging and match up the cache keys. You'll see if the load is started or loaded from memory. Note: preloading only occurs once the user starts scrolling, and even then the first batch of preload only happens when the item at position=0 leaves the top of the screen fully.

  • different view types may need different preloaders. You're declaring 10 view types, but I'm not sure what the possible values for intMediaStoreFileViewType are. A one pixel difference in size of the imageview, because the layout is a tid bit different, can mess up the preloading. See the first and previous point.

  • preloading 200 items sounds crazy, probably doing more harm than good. Just think about it: if the user starts stroking the screen up and down, the preload direction changes so 600 (3 images per item) Glide loads will fire up at each of those directional changes... a value of 5-10 (=1-2 screens worth of items) should be enough. The point of preloading is not to support crazy scrolling like in the video, but to have the illusion that all the images are in memory when scrolling reasonably, even though they're not. Excessive preloading may starve the memory cache because the real loads will fill up the memory and make the cache evict all the time. Also remember that Glide works on 2-4 threads, so loading 600 images would take 150-300 cycles to preload, consider image loading almost sequential. It looks like you're loading local files, so higher number may be ok, but the same amount of network load would be really bad.

  • a single extract method refactor may help readability and code length a lot -> easier maintenance: just create an object (e.g. Part) that encompasses imageViewX and autoResizeTextViewX and has a single method called bind(MediaStoreFile) which contains one of the duplicated if-else codes. If you do this simple refactor your getView could be reduce to:

    // viewHolder stuff
    viewHolderListViewGallery.part1.bind(mediaStoreFileRow3.mediaStoreFile1);
    viewHolderListViewGallery.part2.bind(mediaStoreFileRow3.mediaStoreFile2);
    viewHolderListViewGallery.part3.bind(mediaStoreFileRow3.mediaStoreFile3);

    and you would have zero code duplication.

@sjudd do you have anything that I missed? I think you may have more experience with (pre-)loading image-heavy lists/grids.

@mtst99
Copy link
Author

mtst99 commented Feb 13, 2016

WoW. Now I see why this library is so much powerful, you and sjudd are behind it.
Each line of your explanation is a full tutorial for me. So I need to check them step by step and if I couldn't find out about what you explained, gonna bombard you again with questions.
Today I got my best birthday present from you, despite the fact that i'm still not completed on getting to know how to use glide library properly, you made me happy with your detailed explanation and of course with your great job with this library(List Preloader was a great surprize for me). Happy Valentine's Day and Thank you.

@mtst99
Copy link
Author

mtst99 commented Feb 14, 2016

You should probably change to FixedPreloadSizeProvider and use .override() on drawableRequestBuilder to force Glide to load the same size.

drawableRequestBuilder = Glide
                .with(this)
                .fromString() 
                .override(200, 200) // Example for testing the result and it worked OK
                .dontAnimate() // Items still animate but is not my consideration for now
                .centerCrop() // OK

until now no change in result

You should probably change to FixedPreloadSizeProvider

I don't know how to but this is what I found and created this class but how should I use it?>>

public class FixedPreloadSizeProvider<T> implements ListPreloader.PreloadSizeProvider<T> {
    private final int[] size;

    /**
     * Create a new PreloadSizeProvider with a fixed size.
     * @param width  The width of the preload size
     * @param height The height of the preload size
     */
    public FixedPreloadSizeProvider(int width, int height) {
        this.size = new int[]{width, height};
    }

    @Override
    public int[] getPreloadSize(T item, int adapterPosition, int perItemPosition) {
        return Arrays.copyOf(this.size, this.size.length);
    }
}

@TWiStErRob
Copy link
Collaborator

Assign an instance of it to viewPreloadSizeProvider, change the field type to the interface and it'll be used from adapter.preload().

@mtst99
Copy link
Author

mtst99 commented Feb 14, 2016

Would you provide me the code for it, otherwise I couldn't get it.

@TWiStErRob
Copy link
Collaborator

Diff for "my new adapter" above:

-private final ViewPreloadSizeProvider<String> viewPreloadSizeProvider = new ViewPreloadSizeProvider<>();
+private final ListPreloader.PreloadSizeProvider<String> preloadSizeProvider = new FixedPreloadSizeProvider<>(200, 200);

-viewPreloadSizeProvider.setView(convertView.findViewById(R.id.image_view_1));

-return new ListPreloader<>(this, viewPreloadSizeProvider, maxPreload);
+return new ListPreloader<>(this, preloadSizeProvider, maxPreload);

@mtst99
Copy link
Author

mtst99 commented Feb 14, 2016

Now it works perfectly and performance is smooth and good but needs some more changes as you suggested.

Add Glide.clear(viewHolderListViewGallery.imageViewX) before each setVisibility(GONE).

Did this and there was not any noticeable changes in ListView's behaviour. But it was very nice that you mentioned this behaviour of Glide.

You seem to be emulating a GridView with this adapter, why not just use one? :)

At first I was on RecyclerView with GridLayoutManager and after some performance problems tried LinearLayout manager and loading each item with many views(to prevent expensive layout in layout) like what I do now with ListView and then saw some fast and smooth image gallery apps that are using ListView instead of RecyclerVIew and that ignited my curiosity because RecyclerView is newer and more suggested by professionals. So I decided to at least try once and result was very convincing that something is wrong whether with RecyclerView behaviour or how I implemented it in my project and there was no change in result no matter how I tried different approaches, no success.
I experienced same low quality performance on ListView while I used GridView as it gets layouts as items (I don't know how to set only an ImageView as an item without group it inside a ViewGroup.
I thought that there reason should be Layout inside Layout which I have read many times that is a very expensive implementation specially LinearLayout inside LinearLayout with layout weight assignment.
So I get to the point that best approach for now is using a LinearLayout with ImageViews inside, which resulted in a fast and smooth scrolling. (Even with 12 images on each row).
But I'll not sit relax and I'll search and work on behaviour of RecyclerView until find out what's the exact reason of that weird experience.
For now i'm ok with ListView as my knowledge in programming is not that advanced to dig into source codes.


But I have another question about the way you made use of the class FixedPreloadSizeProvider
How is it that we could use classes this way? >>

private final ListPreloader.PreloadSizeProvider<String> preloadSizeProvider = new FixedPreloadSizeProvider<>(200, 200);

What your approach is called to know more about and Why can't we just call >>

private final FixedPreloadSizeProvider<String> preloadSizeProvider = new FixedPreloadSizeProvider<>(200, 200);

@TWiStErRob
Copy link
Collaborator

Glad to hear you got to a point of "good enough". Good luck with future optimizations.


Thanks for the details about Grid. I didn't notice any particular performance benefit between List and Recycler, the only thing that's different is that Recycler supports much more stuff: viewholder, animation, range updates; it's just a better API. The core of them should be very similar though. I use Recycler throughout my apps and not found any issues. Feel free to mail me if you want me to look at some code (this conversation is fairly out of scope for Glide issues already).

What your approach is called to know more about

You could do that, but it's much more flexible that way. I actually don't know what it's called, but one of the first things that I learned for OO programming is to use the least specific interface that satisfies your needs. Maybe try looking for "polymorphism" or "Liskov Substitution Principle". The implementation doesn't matter as long as it provides the behavior defined by the interface (or base class). For example: List var = new ArrayList() VS List var = new LinkedList(); similarly with Set/HashSet/TreeSet. It's not that important for an isolated case, but if you write some complex code you may write it with a specific collection in mind, say use ArrayList everywhere (parameters, return values, fields). Now let's say you wanted to change it to LinkedList for some reason: you'd have to change all of the interfaces/methods/implementations; it's just a lot of work, especially if all you need is the passed in object to be Iterable (to be used in for(x : list)) or be a Collection (able to use .size()). To project this back to your question, I made a mistake of not doing this originally, and you copied that code; so when I suggested what to change I couldn't just say "replace new ViewPreloadSizeProvider<>() with new FixedPreloadSizeProvider<>(200, 200)", because that wouldn't have compiled. Using PreloadSizeProvider as the type of the field now you're free to change the implementation, the rest of the code doesn't have to change (check ListPreloader's constructor as well). Though you'll probably have to change the type back if you want to call setView on it again... but hopefully you get the point; as always, the answer is "it depends" :)

@mtst99
Copy link
Author

mtst99 commented Feb 15, 2016

Warning: after a call to asBitmap() it'll become BitmapRequestBuilder.

Yes and now I'm trying to prevent Glide to play gif animations and the only solution I found is asBitmap.

private BitmapRequestBuilder<String, TranscodeType> bitmapRequestBuilder;

What is TranscodeType I couldn't find any example about. Thanks

@TWiStErRob
Copy link
Collaborator

https://docs.google.com/drawings/d/1KyOJkNd5Dlm8_awZpftzW7KtqgNR6GURvuF6RfB210g/edit?usp=sharing

TranscodeType is the last R accepted by Targets.

In a DrawableRequestBuilder for most loads Bitmap is the 3rd type and then it's wrapped in a drawable. BitmapRequestBuilder is a little degenerate because it has fixed Bitmap representation, but it's transcoded into a Bitmap as well.

@mtst99
Copy link
Author

mtst99 commented Feb 15, 2016

So I should set a drawble as a second parameter, but as always without example code it's hard to get what should I do.

@mtst99
Copy link
Author

mtst99 commented Feb 15, 2016

TranscodeType is the last R accepted by Targets.

In my case the last R accepted by Targets is viewholder's imageViews inside adapter so how could it be translated into bitmapRequestBuilder parameter? found nothing similar on stackoverflow and on your posts or other pages.

So I changet to this

private BitmapRequestBuilder<String, Drawable> bitmapRequestBuilder;

But I've got error on

 bitmapRequestBuilder = Glide
                    .with(this)
                    .fromString()
                    .asBitmap()
                    .override(200, 200)
                    .centerCrop();

compiler asks me to change Drawable to Bitmap.

@TWiStErRob
Copy link
Collaborator

Try to use the IDE's "Extract variable" refactoring to figure out a type of an expression, just select the expression and execute the refactoring.

Nomen est omen:

  • DrawableRequestBuilder builds a request to end up with a Drawable.
  • BitmapRequestBuilder builds a request to end up with a Bitmap.

The first generic argument to both is the type of the model, that is the type of the object passed to load. The second argument for BitmapRequestBuilder is the transcode type, which is a little counterintuitive, because it loads the bitmap. But a bitmap then can be transcoded to anything, usually to a Bitmap. Sometimes there are weirder cases, see asBitmap().toBytes() for example. This is where the flexibility of Giide comes in (see flow diagram). Your case is the usual one: BitmapRequestBuilder<String, Bitmap>.

@TWiStErRob
Copy link
Collaborator

...RequestBuilder<..., TranscodeType> is only compatible with Target<TranscodeType>, see the signature of GenericRequestBuilder.into(Target). ImageView is always wrapped into ImageViewTarget<TranscodeType> magically (see GenericRequestBuilder.into(ImageView)'s last line), which matches the types up when you just pass in an ImageView.

@mtst99
Copy link
Author

mtst99 commented Feb 24, 2016

Ok. Now I have my own listener that should be added to the ListView while preload listener is assigned to ListView. Problem is when I use a list for MultiScrollListener and add two listener inside that list, performance of ListView (laggy scrolling) decreases drastically.
How could I add my own listener inside preload listener of Glide or is there any better way to assign it to be less expensive?

@mtst99
Copy link
Author

mtst99 commented Feb 24, 2016

This is my multi scroll listener

public class MultiScrollListener implements AbsListView.OnScrollListener {
List<AbsListView.OnScrollListener> mListeners = new ArrayList<>();

public void addScrollListener(AbsListView.OnScrollListener listener) {
    mListeners.add(listener);
}

public void removeListener(AbsListView.OnScrollListener listener) {
    mListeners.remove(listener);
}

@Override
public void onScrollStateChanged(AbsListView view, int scrollState) {
    for (AbsListView.OnScrollListener listener : mListeners) {
        listener.onScrollStateChanged(view, scrollState);
    }
}

@Override
public void onScroll(AbsListView view, int firstVisibleItem, int visibleItemCount, int totalItemCount) {
    for (AbsListView.OnScrollListener listener : mListeners) {
        listener.onScroll(view, firstVisibleItem, visibleItemCount, totalItemCount);
    }
}

}

@TWiStErRob
Copy link
Collaborator

Two things you should look into:

  1. Try your other listener alone, maybe it is the cause of lag by itself if the preloader was ok by itself.
  2. Better iteration inside Multi-listener

for(:) creates an Iterator object which probably trashes your memory and causes unnecessary GC. See Android docs, you can keep the for(:) loop, but change:

private AbsListView.OnScrollListener[] mListeners = new AbsListView.OnScrollListener[0];
public void addScrollListener(AbsListView.OnScrollListener listener) {
    AbsListView.OnScrollListener[] newListeners = Arrays.copyOf(mListeners, mListeners.length + 1);
    newListeners[newListeners.length - 1] = listener; // add last (that place is null in copy)
    mListeners = newListeners; // commit new list
}

(you can also probably get away with not implementing remove; even if not it should be easy to do)

You may get away with for(int i = 0, len = mListeners.size(); i < len; ++i) { mListeners.get(i)... }, try that first before the plain array solution.

@mtst99
Copy link
Author

mtst99 commented Feb 25, 2016

Thank you.

Try your other listener alone, maybe it is the cause of lag by itself if the preloader was ok by itself.

Yes. I checked it out and already did some optimizations on my listener, and if I set preload or my listener alone they are are fine.
With multiListener result is not very good but with replacing the arraylist with array it's much better and smoother. 👍
Is it logical if I replace all the replaceable arrayLists with arrays in my project?
It seems there is a better performance by this approach.

@TWiStErRob
Copy link
Collaborator

It depends... not necessarily worth it, just look at that addScrollListener method... reinventing the wheel. As I said a fori with a .get(i) may be enough in most cases. OnScrollListener is special because it is called many times per second on the UI thread, so it has to be performant. for(:) loops are relatively rare (at least in my codebase) and usually run on background threads once, so those should be ok.

@TWiStErRob
Copy link
Collaborator

You could also specialize your Multi-listener to Dual with two fields (a,b) and implement methods like:

void f(...) {
    a.f(...);
    b.f(...);
}

it's hard to beat that performance-wise :) Though this only works if you don't need the dynamic add/remove.

@mtst99
Copy link
Author

mtst99 commented Feb 25, 2016

Thanks a lot. I'll try it.

@nieldeokar
Copy link

Hey @mtst99 it was great reading i hope you must have finished this things! but i came across similar thing and i am stucked in Recyclerview so can you please update here with your final code? Thanks!

@mtst99
Copy link
Author

mtst99 commented May 13, 2016

Hi @ndeokar . I ended up manage memory on my own and not using third libraries. Much simpler and faster as these libraries meant to support variety of behaviours while I only needed some parts of this library. I didn't probe library with details but I got what I think is logical and of course more smooth. :)

@TWiStErRob
Copy link
Collaborator

@mtst99 it would be interesting to see how you got better performance, maybe we could incorporate it into Glide. Do you think requests and async loads so big overhead?

@mtst99
Copy link
Author

mtst99 commented May 13, 2016

@TWiStErRob sure. I'll share my code as soon as I finish all the levels of preloading and caching for different amount of images in each row 3, 5 and 10.

@nieldeokar
Copy link

@mtst99 That would be great! bdw i must say @TWiStErRob Glide is really powerful ! Thank you for such big help!

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

3 participants