Unnecessary number of finds/fetches caused by .save()?

[ffxsam]

This is for a Meteor method call:

This is from Kadira, and it's all from a single reel.save() call. Why is this happening, and how can I optimize this?

[ffxsam]

Another comparison below. Essentially the same code, but the first image is with using Astronomy, and the second is using typical Mongo collection .update().

[lukejagodzinski]

It's because I've change how the _.isNew property works. Right now it checks if a given document is already stored in collection, so it makes find().count() to figure it out.

[ffxsam]

Hmm, this is kind of a bummer. I'm seeing a performance hit due to this. The screenshots above are for a local dev box, so the numbers are small, but on a real production server, it's more like 400ms-800ms or so. The extra database hits cost, so I might try to limit how much I use Astronomy server-side. If you have any advice on performance optimizations with regards to using Astronomy, I'd be interested in hearing it!

[lukejagodzinski]

Hmmm right I didn't though about server database hits... Locally in deed it will be fast but on the server it may cause problems. Ok I will try to investigate how I can limit number of database hits.

[ffxsam]

I didn't think about it either! I'm so used to just building the app and modeling the DB that I often forget about optimization. Until I launch a service and people are using it.. then I noticed there was considerable latency. 😄 Now I'm obsessively trying to reduce the number of .find() and .fetch() calls used.

[lukejagodzinski]

I will probably have to get back to the old way of storing information in the _isNew property... which is ugly in my opinion, but it's probably the only way to avoid hitting database on _isNew check. I could also introduce two new methods: doc.insert() and doc.update() which would just insert or update document without checking if it's a doc already stored in a collection. However, it could introduce some conflicts with already defined methods by someone in an app. I will just store the _isNew information in some place to make use of it.

[lukejagodzinski]

Ok I've prepared intermediate solution. Astronomy will use the old way of checking without deprecation warning. I've introduce new syntax because of the bug which cause that we couldn't tell if a document is new in the beforeInit event. However I've solved it right now using the old way. So I've removed deprecation warning from the doc._isNew however I'm still leaving Class.isNew(doc). It was actually unnecessary to introduce.

[lukejagodzinski]

I will publish new version tomorrow because I have to create more test, so in the future I will not break anything :)

[ffxsam]

Sounds good! Sorry I didn't have time to come up with a fix, juggling a lot here lately.

[lukejagodzinski]

No problem, it's something that I had to deal with. Good that this problem came out now and not later. I've just forced myself to fix the old bug in a better way.

[ffxsam]

Hey @jagi! I just saw v2.3.9 get published. Does this address what we talked about in this thread?

[lukejagodzinski]

No it does not address this problem yet. I still haven't prepared tests yet. I'm a little bit busy right now with other stuff. I hope I will have time this week to finish this fix and release it.

[lukejagodzinski]

Fixed in 2.3.10

[ffxsam]

So I have 2.3.10 now, and the following code on the server end:

  run({ orgId }) {
    const org = Organisation.findOne(orgId);
     
    delete org.slackConfigUrl;
    delete org.slackIntegration;
    org.save();
  },

This results in the following:

If I change the code to this:

  run({ orgId }) {
    Organisations.update(orgId, {
      $unset: {
        slackConfigUrl: '',
        slackIntegration: '',
      },
    });
  },

The result in Kadira is:

I understand Astronomy is highly complex and there's a lot going on underneath the hood. 8ms vs 28ms is a huge performance difference.. this is local, of course, so in the real world the numbers would be bigger. If there's not much else that can be done to optimize this, I'll probably avoid using Astronomy when fetching data from the server, and only use it when absolutely necessary.

[lukejagodzinski]

Every extra layer influence performance. Of course Astronomy is not perfect for all use cases. I remember reading a programming book years ago about using already made components vs. creating them by hand. It was compared to making an airplane not from already made components but designing everything from scratch: belts, sits, lights, handles etc. Of course it wouldn't be worth an effort. You can always write your programs in assembler - they're gonna be lightning fast :). Probably I could do some more optimisations but it could influence reliability. Astronomy got a lot faster from v1 and it will probably be even faster in v3, as it's probably won't be based on Meteor but it will be GraphQL ready. But it's just a plan. I've been testing several different solutions but don't have a lot of time to make them reality.

[ffxsam]

Totally understandable - it's hard to find free time to contribute to open source projects, I (as well as many others) appreciate what you've built!

Cheers!