iitc-project / ingress-intel-total-conversion

ingress.com/intel total conversion user script with some new features. Should allow easier extension of the intel map.
http://iitc.jonatkins.com/
ISC License
991 stars 552 forks source link

Review script performance #409

Open hastarin opened 11 years ago

hastarin commented 11 years ago

After using IITC heavily for a large operation last weekend I noticed on my desktop PC (the slowest I own) IITC was struggling a bit.

It might be nice to take some time to review the code and see what can be done to improve performance. To that end I did some profiling and noted some areas that might need review. They may already be as optimal as they can, but I thought I'd highlight them for review anyway.

Are portals currently rendered even if the layer isn't displayed? If so can we prevent this?

Below I've just copied/pasted sections of code that were highlighted by the profiler as particularly heavy on the CPU usage.

// renders a portal on the map from the given entity
window.renderPortal = function(ent) {
  if(Object.keys(portals).length >= MAX_DRAWN_PORTALS && ent[0] !== selectedPortal)
    return removeByGuid(ent[0]);
// looks for the GUID in either the layerGroup or entityHash, depending
// on which is faster. Will either return the Leaflet entity or null, if
// it does not exist.
// For example, to find a field use the function like this:
// field = findEntityInLeaflet(fieldsLayer, fields, 'asdasdasd');
window.findEntityInLeaflet = function(layerGroup, entityHash, guid) {
  // fast way
  if(map.hasLayer(layerGroup)) return entityHash[guid] || null;

  // slow way in case the layer is currently hidden
  var ent = null;
  layerGroup.eachLayer(function(entity) {
    if(entity.options.guid !== guid) return true;
    ent = entity;
    return false;
  });
  return ent;
}
window._storedPaddedBounds = undefined;
window.getPaddedBounds = function() {
  if(_storedPaddedBounds === undefined) {
    map.on('zoomstart zoomend movestart moveend', function() 

{
      window._storedPaddedBounds = null;
    });
  }
  if(renderLimitReached(0.7)) return window.map.getBounds();
  if(window._storedPaddedBounds) return 

window._storedPaddedBounds;

  var p = window.map.getBounds().pad(VIEWPORT_PAD_RATIO);
  window._storedPaddedBounds = p;
  return p;
}
$.each(val.gameEntities || [], function(ind, ent) {
      // ent = [GUID, id(?), details]
      // format for links: { controllingTeam, creator, edge }
      // format for portals: { controllingTeam, turret }

      if(ent[2].turret !== undefined) {

        var latlng = [ent[2].locationE6.latE6/1E6, ent[2].locationE6.lngE6/1E6];
        if(!window.getPaddedBounds().contains(latlng)
              && selectedPortal !== ent[0]
              && urlPortal !== ent[0]
              && !(urlPortalLL && urlPortalLL[0] === latlng[0] && urlPortalLL[1] === latlng[1])
          ) return;

        if('imageByUrl' in ent[2] && 'imageUrl' in ent[2].imageByUrl) {
          if(window.location.protocol === 'https:') {
            ent[2].imageByUrl.imageUrl = ent[2].imageByUrl.imageUrl.indexOf('www.panoramio.com') !== -1
                                       ? ent[2].imageByUrl.imageUrl.replace(/^http:\/\/www/, 'https://ssl').replace('small', 'medium')
                                       : ent[2].imageByUrl.imageUrl.replace(/^http:\/\//, '//');
          }
        } else {
          ent[2].imageByUrl = {'imageUrl': DEFAULT_PORTAL_IMG};
        }

        ppp[ent[0]] = ent; // delay portal render
      } else if(ent[2].edge !== undefined) {
        renderLink(ent);
      } else if(ent[2].capturedRegion !== undefined) {
        $.each(ent[2].capturedRegion, function(ind, vertex) {
          if(p2f[vertex.guid] === undefined)
            p2f[vertex.guid] = new Array();
          p2f[vertex.guid].push(ent[0]);
        });
        renderField(ent);
      } else {
        throw('Unknown entity: ' + JSON.stringify(ent));
      }
    });
jonatkins commented 11 years ago

I've been meaning to look into performance myself. The area I was going to look at first was reducing the number of portals rendered, because as things stand now it's practically useless to zoom out when showing a large city/country due to the number of portals shown.

My git feeling on this was that we were pushing the browser limits on the number of entities drawn more than anything else. I wonder if this profiling takes account of time spent outside of the javascript itself (e.g. inside the Leaflet code, where DOM is modified, SVG entities created - and the browser rendering too)

inye commented 11 years ago

@jonatkins it seems that cause of at least one of IITC performance troubles is render limit implementation.

IITC constantly checks number of rendered portals/links/fields using code similar to:

if(Object.keys(portals).length >= MAX_DRAWN_PORTALS) ...

It turns out that complexity of this code is O(n), where n is count of portals! What's worse, JavaScript have no any built-in means of determining number of object keys in constant time. One have to count keys somehow manually.

I've quickly hacked for myself version of IITC without renderLimit at all. It feels much, much smoother than original build. I'm finally able to scroll through portal-dense areas without pressing F5.

vita10gy commented 11 years ago

Could this be swung with a simple counter, or would there be 492394 places ++ing or --ing?

inye commented 11 years ago

@vita10gy it seems it can be done with a simple counter. I've submitted pull request.

vita10gy commented 11 years ago

Obviously it can be disabled, but I'm thinking the show portal levels on portal plugin should turn itself off at some point.

The numbers are pretty pointless once you zoom out even a moderate amount. Cities turn into a portal jumble in as little as 3-4 zooms from as far in as you can go.

I don't know what this would save, but it would be something, and there's kind of no reason not to.

jonatkins commented 11 years ago

It's not just portal numbers - the entire mess of drawn portals when zoomed out is pretty useless.

My ideas for this are to group them together as a new marker - a "portal group".

The portal group marker is used instead of individual portals when there are more than X portals in a Y_Y pixel square on the map. Obviously, would need to determine good values for X and Y here - but perhaps 8_8 pixel squares, and over 4 portals?

Selecting the portal group marker would bring up a list of the individual portals, and some basic stats, in the current portals detail pane. You could then select an individual portal from this list to see its details.

Of course, we still have the issue of having to process a pretty large amount of data from the servers. Ideally the server API would support this portal grouping API directly, so the data is only returned when needed. Not something we have any control over unfortunately.

vita10gy commented 11 years ago

It would also raise some issues with highlighting.

jonatkins commented 11 years ago

The new 0.14.0 has many improvements in performance due to the major rewrite of the portal loading/rendering code. The current test builds have even more improvements in this area.

I decided against the 'portal group' idea. Now, when zoomed out, only some of the portals in the dense areas are added to the map. Yes, some will not be displayed - but in these cases so many were drawn at the same location it was impossible to identify/select individual portals anyway.

It still isn't perfect - zooming out to view, for example, europe or mainland US can end up with tens of thousands of L7+ portals loaded.

If anyone has any further suggestions for improvements in these areas, let me know. Some plugins could probably do with some work too. e.g. scoreboard really struggles with most of europe in view (35000+ portals) - but the best answer may just be a limit on the number of portals they can handle (like max-links does already)