greenfrvr / hashtag-view

Android fully customizable widget for representing data like hashtags collection and similiar.
MIT License
337 stars 54 forks source link

Long hash tag cause the app to be unresponsive #3

Closed lawloretienne closed 9 years ago

lawloretienne commented 9 years ago

If I supply tags with many characters, over 15 characters, it can cause the app to become unresponsive.

ArrayList<String> tags = new ArrayList<>();
tags.add("underwatercinematograpy");
tags.add("surfcinematography");
tags.add("chrisbryanfilms");
HashtagView.setData(tags, Transformers.HASH);

Do you know how this can be fixed?

lawloretienne commented 9 years ago

This seems to be manifesting itself even for some shorter tag lengths. Maybe its the combination of certain tags that causes a repeating garbage collection.

09-24 21:15:06.227 24237-24247/com.etiennelawlor.loop I/art: Background sticky concurrent mark sweep GC freed 532457(16MB) AllocSpace objects, 0(0B) LOS objects, 19% free, 59MB/73MB, paused 9.164ms total 85.730ms

Here is another combination of tags that triggered the repeating garbage collection :

teahupoo tahiti raimana surfing surfer waves wipeout 4k xxl dji phantom3 inspire1

greenfrvr commented 9 years ago

Can you tell me widget width or screen sizes (if you use match_parent), so that I could reproduce this issue? I guess there's no quick fix, so I'll try to solve this problem in the closest update.

lawloretienne commented 9 years ago

I am using a Nexus 5 to test this. The android:layout_width="match_parent" .

greenfrvr commented 9 years ago

That's strange, I tested on the same device with same attributes, and the result is fine. I mean all items are displayed as they should and allocation tracker doesn't detect any memory leaks. Probably the problem is not in the library. By the way, thanks, you just pointed out on one potential bug.

lawloretienne commented 9 years ago

If you tested both of those scenarios, then I'm not sure what else I can give you to provide some more context on the issue. Do you have any ideas?

lawloretienne commented 9 years ago

I updated the dependency to :

compile 'com.github.greenfrvr:hashtag-view:1.1.0'

greenfrvr commented 9 years ago

There is an issue#4 I have just opened, it causes behavior you described, but if all items are displayed fine than memory leaks appear somewhere in other place. Below I attach screenshot with your data tests:

screenshot_2015-09-25-10-26-02

lawloretienne commented 9 years ago
screen shot 2015-09-25 at 12 57 57 am

The memory monitor looks like that. Repeated garbage collection.

lawloretienne commented 9 years ago

What if you test it with just the three tags

tags.add("underwatercinematograpy");
tags.add("surfcinematography");
tags.add("chrisbryanfilms");

and not all the other tags?

greenfrvr commented 9 years ago

Have you captured that with allocation tracker? This can help to investigate what causes that beahaviour.

greenfrvr commented 9 years ago

I tested both cases you described. On screenshot there are two separeate widgets: first contains three items "underwatercinematograpy", "surfcinematography", "chrisbryanfilms", second contains all the rest.

lawloretienne commented 9 years ago

screenshot_20150925-011129

Okay so I was originally applying a 16dp padding to the parent of the HashtagView, which was causing the issue. When I removed this, then it loads fine and doesn't do the garbage collection.

lawloretienne commented 9 years ago

So i guess that https://github.com/greenfrvr/hashtag-view/issues/4 is in fact an issue. There must be some kind of layout calculation that is happening that gets into an infinite loop.

lawloretienne commented 9 years ago

The infinite loop is happening in the sort() method which is called in the preDrawListener.

greenfrvr commented 9 years ago

Right, then I know how to solve it, and will try to fix it in a few days.

lawloretienne commented 9 years ago

I'm not sure this is the best way to go about this, but you could use a flag to check if an item was removed from data. If you go through a pass and no items can be removed from data, then those item widths must be larger than the ViewWidth and therefore they cannot be displayed.

private void sort() {
    if (data == null || data.isEmpty()) return;
    int rowsQuantity = evaluateRowsQuantity();
    final int[] rowsWidth = new int[rowsQuantity];
    viewMap = ArrayListMultimap.create(rowsQuantity, data.size());
    while (!data.isEmpty()) {
        boolean somethingRemoved = false;
        rowIteration: for (int i = 0; i < rowsQuantity; i++) {
            for (ItemData item: data) {
                if (rowsWidth[i] + item.width < getViewWidth()) {
                    rowsWidth[i] += item.width;
                    viewMap.put(i, item);
                    data.remove(item);
                    somethingRemoved = true;
                    continue rowIteration;
                }
            }
        }
        if (!somethingRemoved) {
            break;
        }
    }
}
greenfrvr commented 9 years ago

In fact infinite loop is appearing in evaluateRowsQuantity() method, when item is wider than widget, it can't be inserted in any row. So I see two ways to solve this:

lawloretienne commented 9 years ago

I see you have made several commits recently. Are you going to push out an updated version to Maven (v 1.2.0) ?

greenfrvr commented 9 years ago

yeap, need to add one feature and will be ready to update to version 1.1.1.

lawloretienne commented 9 years ago

thats awesome. thanks for being so responsive. and you really have created a great library.