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

Assorted fixes for recently introduced issues. #1405

Merged
merged 3 commits into from
Feb 4, 2023

Conversation

linas
Copy link
Member

@linas linas commented Feb 1, 2023

Per discussion on #1403, and especially comment
#1403 (comment) and also
#1403 (comment)

@linas linas mentioned this pull request Feb 1, 2023
@ampli
Copy link
Member

ampli commented Feb 1, 2023

In any case, I didn't find an influence of the table element pool chunk size on the performance of ru (which is significantly worse than in the current release). I also tried with an INV_LOAD_FACTOR of 10 (as before), and it didn't help.

In addition, neutralize the extra sorting in VDAL_compare_linkages() and it also didn't help the ru performance.
So I will continue to investigate that (and the apparent slowdown for sentences with null_count > 0).

Regarding MAX_LOG2_TABLE_SIZEm maybe it would be better to move the constants to #define (I can do it the next time I add a #deine).

@linas
Copy link
Member Author

linas commented Feb 1, 2023

INV_LOAD_FACTOR

I'm thinking that setting it to 2 would be fine. There's already a lot of padding in the size-estimate. Possibly even 1 would be harmless. I'll try to measure this "soon".

it would be better to move the constants to #define

I assume you mean dict #define For this table, it does not matter. However, for the Parse_choice it could solve the issue I'm facing. But also #1406 could also alleviate much or most of the concerns.

@ampli
Copy link
Member

ampli commented Feb 1, 2023

I'm thinking that setting it to 2 would be fine. There's already a lot of padding in the size-estimate. Possibly even 1 would be harmless. I'll try to measure this "soon".

A high load factor may prevent a timely table groth.
It also leads to frequent CPU cache misses in the linear table slots scan (until the value or NULL are found).
I have a branch that implements the Robin-hood cache placement strategy but I didn't observe a speedup.

@linas
Copy link
Member Author

linas commented Feb 1, 2023

A high load factor may prevent a timely table growth.

It might, but what I am saying is that there is also a "hidden" load factor in the initial size estimate, which is already a factor of 2 or three for the "typical" case.

@linas linas merged commit f64048b into opencog:master Feb 4, 2023
@linas linas deleted the comment-fixes branch February 4, 2023 19:28
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