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

Make check_symbols not accept dynamic c++ symbols #181

Merged
merged 2 commits into from
Sep 26, 2023

Conversation

Ngalstyan4
Copy link
Contributor

Turns out our static usearch library still depended on the cpp runtime library libstdc++ dynamically.
This usually did not cause issues since postgres
commonly links libstdc++ dynamically so c++ symbols in our .so would be resolved dynamically, when loaded.

This did cause issues for when postgres was not linked with libstdc++. I do not know when exactly this happens but it seems this at least happens when building postgres from source under certain circumstances

This commit makes sure that we do not use libstdc++ symbols from postgres dynamic library dependency and actually enforce that all c++ symbols we require are in the sahred object we build

@Ngalstyan4 Ngalstyan4 requested a review from var77 September 26, 2023 10:08
@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

Merging #181 (bdcb8bc) into main (39b5b00) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #181   +/-   ##
=======================================
  Coverage   83.29%   83.29%           
=======================================
  Files          17       17           
  Lines        1161     1161           
  Branches      245      245           
=======================================
  Hits          967      967           
  Misses         85       85           
  Partials      109      109           

@github-actions
Copy link

github-actions bot commented Sep 26, 2023

Benchmarks

metric old new pct change
recall (after create) 0.740 0.740 -
recall (after insert) 0.738 0.726 -1.63%
select bulk tps 214.972 217.193 +1.03%
select bulk latency (ms) 36.515 36.122 -1.08%
select bulk latency (stddev ms) 8.139 8.295 +1.92%
create latency (ms) 1497.447 1489.314 -0.54%
insert bulk tps 14.138 13.927 -1.49%
insert bulk latency (ms) 70.718 71.789 +1.51%
insert bulk latency (stddev ms) 3.203 2.440 -23.82%
disk usage (bytes) 6348800.000 6348800.000 -

Turns out our static usearch library still depended on the cpp runtime
library libstdc++ dynamically.
This usually did not cause issues since postgres
commonly links libstdc++ dynamically so c++ symbols in our .so would be
resolved dynamically, when loaded.

This did cause issues for when postgres was not linked with libstdc++. I
do not know when exactly this happens but it seems this at least happens
when building postgres from source under certain circumstances

This commit makes sure that we do not use libstdc++ symbols from
postgres dynamic library dependency and actually enforce that all c++
symbols we require are in the sahred object we build
@Ngalstyan4 Ngalstyan4 force-pushed the narek/static-libstdc++ branch from 4bd1e19 to bdcb8bc Compare September 26, 2023 16:25
@Ngalstyan4 Ngalstyan4 merged commit 7e6223e into main Sep 26, 2023
@Ngalstyan4 Ngalstyan4 deleted the narek/static-libstdc++ branch September 26, 2023 16:48
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