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

Change SweepEvent structure #26

Closed
mfogel opened this issue Jul 22, 2018 · 1 comment
Closed

Change SweepEvent structure #26

mfogel opened this issue Jul 22, 2018 · 1 comment
Labels
performance Possible performance improvement
Milestone

Comments

@mfogel
Copy link
Owner

mfogel commented Jul 22, 2018

Here's the relevant data structure as it exists now:

class SweepEvent {
  // a pointObject is a simple object of form {'x': <number>, 'y': <number>}
  // it is shared with normally one, but possibly more, other SweepEvents
  point: pointObject,

  // other stuff...
}

Here's the structure I think it would be worth testing:

class SweepEvent {
  // flattened point object. No more sharing of these coordinates between SweepEvents with
  // the same points - the raw numbers would just be repeated.
  x: <number>,
  y: <number>,

  // other stuff...
}

In addition, those point objects {'x': <number>, 'y': <number>} are passed to many helper methods/functions. Those could all be changed to just accept two separate arguments: x: <number> and y: <number>.

This could slightly increase memory usage (or decrease it? unsure how much overhead the small point objects introduce in JS) but would theoretically decrease the number of steps of dereferencing pointers to get at the values.

@mfogel mfogel added the enhancement New feature or request label Jul 22, 2018
@mfogel mfogel added performance Possible performance improvement and removed enhancement New feature or request labels Aug 5, 2018
@mfogel mfogel changed the title Possible performance optimization: flatten point structure Flatten point structure Aug 5, 2018
@mfogel
Copy link
Owner Author

mfogel commented Sep 18, 2018

After testing this out, it appears that this decreases performance in most cases. Re-using the point object is better than flattening it.

In fact, better to move all information shared between sweep events into a shared object. This means that the current structure:

class SweepEvent {
  point: {
    x: <number>,
    y: <number>
  }
  linkedEvents: [<SweepEvent>, ... ]
  segment: <Segment>
}

Would be more performant as:

class SweepEvent {
  point: {
    x: <number>,
    y: <number>,
    events: [<SweepEvent>, ... ]
  }
  segment: <Segment>
}

@mfogel mfogel changed the title Flatten point structure Change SweepEvent structure Sep 18, 2018
@mfogel mfogel closed this as completed in babe799 Sep 18, 2018
@mfogel mfogel added this to the 0.9 milestone Oct 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Possible performance improvement
Projects
None yet
Development

No branches or pull requests

1 participant