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

Testcase for Issue 72: LayerNodeIndex:add() performance #73

Merged
merged 2 commits into from Jan 1, 2013

Conversation

Projects
None yet
5 participants
@jonathanwin
Contributor

jonathanwin commented Nov 14, 2012

No description provided.

@peterneubauer

This comment has been minimized.

Show comment
Hide comment
@peterneubauer

peterneubauer Nov 15, 2012

Contributor

@jonathanwin great work Jonathan! In order to pull this in, could you please send in the CLA, http://docs.neo4j.org/chunked/snapshot/cla.html ?

Contributor

peterneubauer commented Nov 15, 2012

@jonathanwin great work Jonathan! In order to pull this in, could you please send in the CLA, http://docs.neo4j.org/chunked/snapshot/cla.html ?

@jonathanwin

This comment has been minimized.

Show comment
Hide comment
@jonathanwin

jonathanwin Nov 16, 2012

Contributor

---------- Forwarded message ----------
From: Jonathan Winterflood jonathan.winterflood@gmail.com
Date: Thu, Nov 15, 2012 at 3:15 PM
Subject: CLA
To: cla@neotechnology.com

Hi. My name is Jonathan Winterflood (jonathan.winterflood@gmail.com).
I agree to the terms in the attached Neo4j Contributor License Agreement.

Jonathan

Contributor

jonathanwin commented Nov 16, 2012

---------- Forwarded message ----------
From: Jonathan Winterflood jonathan.winterflood@gmail.com
Date: Thu, Nov 15, 2012 at 3:15 PM
Subject: CLA
To: cla@neotechnology.com

Hi. My name is Jonathan Winterflood (jonathan.winterflood@gmail.com).
I agree to the terms in the attached Neo4j Contributor License Agreement.

Jonathan

@peterneubauer

This comment has been minimized.

Show comment
Hide comment
@peterneubauer

peterneubauer Nov 16, 2012

Contributor

Great, will try to merge today if I get the time!

Contributor

peterneubauer commented Nov 16, 2012

Great, will try to merge today if I get the time!

@craigtaverner craigtaverner merged commit 33d65ea into neo4j-contrib:master Jan 1, 2013

@craigtaverner

This comment has been minimized.

Show comment
Hide comment
@craigtaverner

craigtaverner Jan 1, 2013

Contributor

I merged in the test case, and then disabled it until we can actually fix the performance bug. This should be done in issue 72, so we can consider issue 73 completed (pull request accepted).

Contributor

craigtaverner commented Jan 1, 2013

I merged in the test case, and then disabled it until we can actually fix the performance bug. This should be done in issue 72, so we can consider issue 73 completed (pull request accepted).

@peterneubauer

This comment has been minimized.

Show comment
Hide comment
@peterneubauer

peterneubauer Jan 2, 2013

Contributor

Awesome.
Thanks Craig!

On Tue, Jan 1, 2013 at 11:22 PM, Craig Taverner notifications@github.comwrote:

I merged in the test case, and then disabled it until we can actually fix
the performance bug. This should be done in issue 72, so we can consider
issue 73 completed (pull request accepted).


Reply to this email directly or view it on GitHubhttps://github.com/neo4j-contrib/spatial/pull/73#issuecomment-11794286.

Contributor

peterneubauer commented Jan 2, 2013

Awesome.
Thanks Craig!

On Tue, Jan 1, 2013 at 11:22 PM, Craig Taverner notifications@github.comwrote:

I merged in the test case, and then disabled it until we can actually fix
the performance bug. This should be done in issue 72, so we can consider
issue 73 completed (pull request accepted).


Reply to this email directly or view it on GitHubhttps://github.com/neo4j-contrib/spatial/pull/73#issuecomment-11794286.

@jonathanwin

This comment has been minimized.

Show comment
Hide comment
@jonathanwin

jonathanwin Jan 4, 2013

Contributor

Constant is most certainly impossible (finding the position of an element
in a set is surely dependent on the size of the set, barring massive memory
trade-offs)

I imagine an r-tree is something like o(log(n)) (ignoring node overflows).
And as far as I know, the spatial r-tree itself does not have a performance
problem.
The problem here is that in front of the r-tree there is a o(n) test, and a
o(n) insert into a plain linked list of all indexed node ids.

This could probably be replaced by a second tree on the indexed ids,
yielding an overall o(log(n)). Or maybe something else...

Jonathan

On Fri, Jan 4, 2013 at 2:23 PM, amorgner notifications@github.com wrote:

What is the expected curve of insert performance? Constant rate?


Reply to this email directly or view it on GitHubhttps://github.com/neo4j-contrib/spatial/pull/73#issuecomment-11882589.

Contributor

jonathanwin commented Jan 4, 2013

Constant is most certainly impossible (finding the position of an element
in a set is surely dependent on the size of the set, barring massive memory
trade-offs)

I imagine an r-tree is something like o(log(n)) (ignoring node overflows).
And as far as I know, the spatial r-tree itself does not have a performance
problem.
The problem here is that in front of the r-tree there is a o(n) test, and a
o(n) insert into a plain linked list of all indexed node ids.

This could probably be replaced by a second tree on the indexed ids,
yielding an overall o(log(n)). Or maybe something else...

Jonathan

On Fri, Jan 4, 2013 at 2:23 PM, amorgner notifications@github.com wrote:

What is the expected curve of insert performance? Constant rate?


Reply to this email directly or view it on GitHubhttps://github.com/neo4j-contrib/spatial/pull/73#issuecomment-11882589.

@amorgner

This comment has been minimized.

Show comment
Hide comment
@amorgner

amorgner Jan 4, 2013

Member

I'm currently working on a fix. Seems much, much easier, with some hints I got from Craig here:
https://groups.google.com/forum/#!topic/neo4j/o07iBeHI8TU

Member

amorgner commented Jan 4, 2013

I'm currently working on a fix. Seems much, much easier, with some hints I got from Craig here:
https://groups.google.com/forum/#!topic/neo4j/o07iBeHI8TU

@amorgner

This comment has been minimized.

Show comment
Hide comment
@amorgner

amorgner Jan 4, 2013

Member

As this issue is closed, we should continue in #72

Member

amorgner commented Jan 4, 2013

As this issue is closed, we should continue in #72

@ehx-v1

This comment has been minimized.

Show comment
Hide comment
@ehx-v1

ehx-v1 May 9, 2016

Contributor

you mean #82

Contributor

ehx-v1 commented May 9, 2016

you mean #82

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment