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

Possible constructor search optimization #6

Open
andrewdep opened this issue Feb 28, 2022 · 3 comments
Open

Possible constructor search optimization #6

andrewdep opened this issue Feb 28, 2022 · 3 comments

Comments

@andrewdep
Copy link

Since JS Maps and Sets support objects for keys, I'm wondering if you've done any benchmarking to see if using a Map instead of a linear array search using an iterator would optimize this code:

  itemToSia(item, constructor) {
    for (const entry of this.constructors) {
      if (entry.constructor === constructor) {
        return {
          code: entry.code,
          args: entry.args(item),
        };
      }
    }
    throw `Serialization of item ${item} is not supported`;
  }

What I'm thinking is populating the map up front using the constructor property as a key and having the entry be the value, then changing the above code to something like this:

  itemToSia(item, constructor) {
    const entry = this.constructors.get(constructor);
    if (entry) {
      return {
        code: entry.code,
        args: entry.args(item),
      };
    }
    throw `Serialization of item ${item} is not supported`;
  }

I ask because in our project we are up to around 20 custom constructors. I imagine there is a chance this won't be an optimization depending on modern javascript compilers, but technically the of is creating iterator objects every time, along with the fact that iterators return short lived objects for every element iterated (though, again, modern compilers may optimize this away), and its possible that for small Maps iterating is indeed faster than a Map lookup, but I would be curious what benchmarking shows?

@CEbbinghaus
Copy link

CEbbinghaus commented Jan 15, 2023

I have found with large datasets that the

for(let i = 0; i < arr.length; ++i) {
	const data = arr[i];
}

is fastest when iterating objects. However, the question in this issue would be how JS evaluates objects as keys and wether that is faster. It would be worth to run a few benchmarks and see

@c7x43t
Copy link

c7x43t commented Apr 28, 2023

It would be possible to do:

// replace
this.constructors = constructors;
// with
this.constructors = new Map(constructors.map(constructor=>[constructor.constructor,constructor]));
// in Sia:
itemToSia(item, constructor) {
	let entry = this.constructors(constructor);
	if(entry){
		return {
			code: entry.code,
			args: entry.args(item),
		};
	}
	throw `Serialization of item ${item} is not supported`;
}

@pouya-eghbali
Copy link
Owner

These are all great ideas; I will benchmark these and get the best solution into the code. I'm building a big project on this, so I will be working on it more often :) Thank you all for your active interest and help! I merged the pull requests and made a new minor release.

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

4 participants