exoplatform / chat-application

Chat application
GNU Affero General Public License v3.0
38 stars 57 forks source link

optimisations #89

Open gpicavet opened 8 years ago

gpicavet commented 8 years ago

Hello,

I think the app performance could benefit from stopping sending useless requests, and it would be done at low cost on client side :

Besides that, what your toughts on using websockets instead of polling ? It could be interesting but when i look at Google and Facebook, they dont use websockets and continue to use long polling (maybe due to some lack of reliabily) ! So Is there a real use case (intranets limited chat, smaller company, high latenty networks) ? Or should we only rely on some of the http/2 improvements (reduced handshake, header size) and keep doing (fast/long) polling ?

thomasdelhomenie commented 8 years ago

Hello,

I agree there are some optimisations to do. Do not hesitate to send a PR ;-) That said, we have to keep notifications even if the tab is in background. We have started to work on Desktop notifications (https://jira.exoplatform.org/browse/PFR-1043 - and the spec : https://community.exoplatform.com/portal/g/:spaces:platform_4/platform_4/wiki/Chat_Notifications).

About websockets, this is our big next task on the chat application. We have to study how it fits with the chat application. Feel free to give your opinion in Jira issues also !

gpicavet commented 8 years ago

Yes indeed, it makes sense to keep notifications requests in background if we want desktop notif :) I'll try to post a PR as soon as possible, it could be an improvement before a websocket version comes out.

gpicavet commented 8 years ago

Hello, "whoisonline" is another service that scale much more if we replace the multiple selects that get the unread notifications with a single aggregate query.

Note that problems appeared when i have 500+ users each with 50 rooms (80% users, 20% space) refreshing every 5 seconds (100 request/s)

Here's the modifications i've done, response times are now 5 times better on a 2-shards database :

NotificationServiceImpl.java :

  public Map<String, Integer> getUnreadNotificationsTotals(String user, String type, String category, String[] categoryIds, String dbName)
  {
      HashMap<String, Integer> res = new HashMap<String, Integer>();

    DBCollection coll = db(dbName).getCollection(M_NOTIFICATIONS);
    BasicDBObject query = new BasicDBObject();

    query.put("user", user);
//    query.put("isRead", false);
    if (type!=null) query.put("type", type);
    if (category!=null) query.put("category", category);
    if (categoryIds!=null && categoryIds.length>0) query.put("categoryId", new BasicDBObject("$in", categoryIds));

    BasicDBObject group = new BasicDBObject();
    group.put("_id", "$categoryId");
    group.put("count", new BasicDBObject("$sum", 1));

    List<DBObject> pipeline = java.util.Arrays.asList(
            (DBObject)new BasicDBObject("$match", query),
            (DBObject)new BasicDBObject("$group", group));

    for(DBObject doc : coll.aggregate(pipeline).results()) {
        res.put(doc.get("_id").toString(), (Integer)doc.get("count"));
    }

    return res;
  }

ChatServiceImpl.java :

  public List<RoomBean> getExistingRooms(String user, boolean withPublic, boolean isAdmin, NotificationService notificationService, TokenService tokenService, String dbName)
  {
    List<RoomBean> rooms = new ArrayList<RoomBean>();
    String roomId = null;
    DBCollection coll = db(dbName).getCollection(M_ROOMS_COLLECTION);

    BasicDBObject basicDBObject = new BasicDBObject();
    basicDBObject.put("users", user);

    DBCursor cursor = coll.find(basicDBObject);
    while (cursor.hasNext())
    {
      DBObject dbo = cursor.next();
      roomId = dbo.get("_id").toString();
      long timestamp = -1;
      if (dbo.containsField("timestamp")) {
        timestamp = ((Long)dbo.get("timestamp")).longValue();
      }
      List<String> users = ((List<String>)dbo.get("users"));
      users.remove(user);
      if (users.size()>0 && !user.equals(users.get(0)))
      {
        String targetUser = users.get(0);
        boolean isDemoUser = tokenService.isDemoUser(targetUser);
        if (!isAdmin || (isAdmin && ((!withPublic && !isDemoUser) || (withPublic && isDemoUser))))
        {
          RoomBean roomBean = new RoomBean();
          roomBean.setRoom(roomId);
          //roomBean.setUnreadTotal(notificationService.getUnreadNotificationsTotal(user, "chat", "room", roomId, dbName));
          roomBean.setUnreadTotal(0);
          roomBean.setUser(users.get(0));
          roomBean.setTimestamp(timestamp);
          rooms.add(roomBean);
        }
      }
    }

    String[] roomIds = new String[rooms.size()];
    for(int i=0; i<rooms.size();i++) {
        roomIds[i] = rooms.get(i).getRoom();
    }
    Map<String, Integer> notifs = notificationService.getUnreadNotificationsTotals(user, "chat", "room", roomIds, dbName);
    for(RoomBean r : rooms) {
        Integer n = notifs.get(r.getRoom());
        if(n != null)
            r.setUnreadTotal(n);
    }

    return rooms;
  }
thomasdelhomenie commented 8 years ago

Thanks @grisha78 . I will create a PR based on your changes as soon as I can. If you have the time to do the PR in the meantime, do not hesitate ;)

gpicavet commented 8 years ago

Hello, thanks as i have some free time this week i will do it :) Is it more convenient to make a branch per optim (read optim will affect portlet) ?

thomasdelhomenie commented 8 years ago

Good to hear :) Yes, one branch and one PR per optim, it will be easier and quicker to validate, so quicker to land in the product.