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

BITSET index: salloc_index_to_ptr is broken in 1.5 and 1.6 #49

Closed
rtsisyk opened this issue Oct 4, 2013 · 6 comments
Closed

BITSET index: salloc_index_to_ptr is broken in 1.5 and 1.6 #49

rtsisyk opened this issue Oct 4, 2013 · 6 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@rtsisyk
Copy link
Contributor

rtsisyk commented Oct 4, 2013

salloc_index_to_ptr is broken because tuple pointer is shifted by tuple_alloc

1.5 and 1.6 will likely require different fixes

@rtsisyk
Copy link
Contributor Author

rtsisyk commented Oct 7, 2013

I promised to put some light on this bug.
tuple_alloc shifts a tuple pointer and uses this extra space to store field_map:

    char *ptr = (char *) salloc(total, "tuple");
    struct tuple *tuple = (struct tuple *)(ptr + format->field_map_size);

salloc_index_to_ptr always returns the original pointer.
There is no way to get shifted pointer from original because the size of field_map cannot be determined.

@kostja
Copy link
Contributor

kostja commented Oct 12, 2013

Roman, why do tests pass in 1.5? What's wrong with tests that they don't verify this? 1.5 needs a test case to reproduce the problem. Then, for now, simply disable salloc_ptr_to_index in 1.5. In 1.6 we will solve this after adding msgpack.

@rtsisyk
Copy link
Contributor Author

rtsisyk commented Oct 14, 2013

test/box/bitset.lua works with (num, num) tuples. Such kind of tuples do not need the offset table, because offsets can be precalculated and saved in the tuple format definition.

BITSET need a (tuple <-> some_number_id) mapping where the right space is very dense.
ptr_to_index is not so good approximation to the ideal mapping, but it was best that I could implement in our architecture. Replacing ptr_to_index with tuple pointer make BITSET very sparse and therefore ineffective.

Possible ways to fix the bug:

  1. Remove the ugly hack which shifting pointers and move all offsets to the end of tuple
  2. Try to find a better way to enumerate tuples in the space

@rtsisyk
Copy link
Contributor Author

rtsisyk commented Nov 26, 2013

I added a workaround for 1.5 to fix bitset.
Anyway, this bug is not fixed.

@rtsisyk rtsisyk modified the milestones: 1.6.3, 1.6.2 Mar 5, 2014
@kostja kostja modified the milestones: 1.6.4, 1.6.3 May 22, 2014
@kostja
Copy link
Contributor

kostja commented Aug 5, 2014

I think we need to abandon allocator-based pointer translation, and store a translation hash in the bitset itself. The id in the bitset would be simply the offset in the hash with linear collision resolution.
More memory used, but significantly less headache.
Thoughts?

@rtsisyk
Copy link
Contributor Author

rtsisyk commented Aug 6, 2014

I'm not sure about offset in mhash, but I totally agree with the idea.
This ticket should be re-scheduled to 1.5.x.

@rtsisyk rtsisyk self-assigned this Aug 6, 2014
@kostja kostja changed the title salloc_index_to_ptr is broken in 1.5 and 1.6 bitset index: salloc_index_to_ptr is broken in 1.5 and 1.6 Oct 2, 2014
@kostja kostja modified the milestones: 1.6.5, 1.6.4 Oct 10, 2014
@kostja kostja assigned alyapunov and unassigned rtsisyk Apr 23, 2015
@kostja kostja changed the title bitset index: salloc_index_to_ptr is broken in 1.5 and 1.6 BITSET index: salloc_index_to_ptr is broken in 1.5 and 1.6 Apr 23, 2015
alyapunov added a commit that referenced this issue Jun 1, 2015
*) fixed bsize() method of bitset index
@kostja kostja added the master label Jun 3, 2015
@kostja kostja closed this as completed in 9313b4f Jun 22, 2015
drakonhg pushed a commit that referenced this issue Sep 2, 2021
drakonhg pushed a commit that referenced this issue Sep 2, 2021
Unit tests fails because gdbserver appends its own output to stderr.
Consider [1] for possible fix on gdbserver side. Alternativelly it can
be worked around using --wrapper option, I guess.

[1]: https://cygwin.com/ml/gdb-patches/2015-03/msg01051.html

Related to #49.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants