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

Add option that restores the ability to perform index compression in memory #580

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gustingonzalez
Copy link
Collaborator

This adds the --in-memory option to the compress_inverted_index command, restoring the in-memory compression and allowing it to avoid using an intermediate buffer.

Copy link
Member

@elshize elshize left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contributing! Please see the comments about the tests.

Feel free to disregard the failing format test -- seems nothing to do with your change, not entirely sure why it's failing, I'll look into it separately.

test/test_compress.cpp Outdated Show resolved Hide resolved
test/test_compress.cpp Outdated Show resolved Hide resolved
@elshize elshize self-requested a review April 14, 2024 00:03
Copy link

codecov bot commented Apr 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.27%. Comparing base (ce97f7b) to head (e8d7d76).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #580   +/-   ##
=======================================
  Coverage   93.27%   93.27%           
=======================================
  Files          89       89           
  Lines        4448     4448           
=======================================
  Hits         4149     4149           
  Misses        299      299           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gustingonzalez gustingonzalez marked this pull request as draft April 14, 2024 02:31
@gustingonzalez
Copy link
Collaborator Author

@elshize, done with the comments, but I'll be making some minor code refactoring as well.

@gustingonzalez gustingonzalez changed the title Add option that restores the ability to perform index compression in memory. Add option that restores the ability to perform index compression in memory Apr 14, 2024
test/test_compress.cpp Outdated Show resolved Hide resolved
@gustingonzalez
Copy link
Collaborator Author

@elshize, I've noticed a few tests are failing. This comes from the master branch, so it might be worth considering as another issue. Here's the error:

The following tests FAILED:
	  8 - test_block_codecs:Example test case - pisa::optpfor_block (ILLEGAL)
	 10 - test_block_codecs:Example test case - pisa::streamvbyte_block (ILLEGAL)
	 11 - test_block_codecs:Example test case - pisa::maskedvbyte_block (ILLEGAL)
	 17 - test_block_codecs:Example test case - pisa::simdbp_block (ILLEGAL)
	 20 - test_block_codecs:Property test - pisa::streamvbyte_block (ILLEGAL)
	 21 - test_block_codecs:Property test - pisa::maskedvbyte_block (ILLEGAL)
	 27 - test_block_codecs:Property test - pisa::simdbp_block (ILLEGAL)
	 28 - test_block_freq_index:block_freq_index (ILLEGAL)
	 29 - test_block_posting_list:block_posting_list (ILLEGAL)
	 30 - test_block_posting_list:block_posting_list_reordering (ILLEGAL)
	 39 - test_compress:Compress index (ILLEGAL)
	 40 - test_compress:Compress quantized index (ILLEGAL)
	115 - test_stream_builder:Stream builder for block index (ILLEGAL)

@elshize
Copy link
Member

elshize commented Apr 14, 2024

Tests seem to be working fine in CI. What system are you running this on? What compiler? Can you show the output of one of the test? E.g.:

./test/test_block_codecs

@gustingonzalez
Copy link
Collaborator Author

gustingonzalez commented Apr 14, 2024

Tests seem to be working fine in CI. What system are you running this on? What compiler? Can you show the output of one of the test? E.g.:

./test/test_block_codecs

@elshize, effectively there is a problem with my compiler. I tested again compiling a debug version, and it worked fine. I have to look the reasons of this, but at the moment this isn't blocking for me. The error is "ILLEGAL INSTRUCTION", I'm using g++11:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test_block_codecs is a Catch v2.13.7 host application.
Run with -? for options

-------------------------------------------------------------------------------
Example test case - pisa::optpfor_block
-------------------------------------------------------------------------------
./test/test_block_codecs.cpp:67
...............................................................................

./test/test_block_codecs.cpp:81: FAILED:
due to a fatal error condition:
  SIGILL - Illegal instruction signal

===============================================================================
test cases: 1 | 1 failed
assertions: 1 | 1 failed

[1]    20027 illegal hardware instruction (core dumped)  ./test/test_block_codecs

@elshize
Copy link
Member

elshize commented Apr 14, 2024

@gustingonzalez is it possible you previously compiled the code on one machine and executed it on a different one?

@gustingonzalez
Copy link
Collaborator Author

@gustingonzalez is it possible you previously compiled the code on one machine and executed it on a different one?

Great! That was the reason!

@gustingonzalez gustingonzalez marked this pull request as ready for review April 22, 2024 01:54
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

Successfully merging this pull request may close these issues.

2 participants