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

optimisations #89

Open
gpicavet opened this issue Mar 29, 2016 · 6 comments
Open

optimisations #89

gpicavet opened this issue Mar 29, 2016 · 6 comments

Comments

@gpicavet
Copy link
Contributor

Hello,

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

  • only send read request when there's a notification (returning notifications by room along with the total amount), and only returns the last messages
  • stop sending xhr requests when tab is in background (using visible api) and refresh immediately when restoring tab. But it could be interesting to keep notification request as well

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

gpicavet commented Apr 5, 2016

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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.

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

No branches or pull requests

2 participants