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

Would like to free Exp_pool earlier. #1413

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions link-grammar/parse/preparation.c
Original file line number Diff line number Diff line change
Expand Up @@ -195,20 +195,40 @@ static void create_wildcard_word_disjunct_list(Sentence sent,
*/
void prepare_to_parse(Sentence sent, Parse_Options opts)
{
size_t i;

if (IS_GENERATION(sent->dict))
create_wildcard_word_disjunct_list(sent, opts);

build_sentence_disjuncts(sent, opts->disjunct_cost, opts);


#ifdef NEED_THESE_FOR_NULL_AND_PANIC_PARSING
Copy link
Member

Choose a reason for hiding this comment

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

We can add a parse option to indicate that a second pass is not expected. In that case, we can not only free the Exp_pool but immediately delete all the pools that are now pool-reused.

If a second pass happens anyway (disregarding the value of the said parse option) and there are no expressions, we can just return an error.

Copy link
Member Author

Choose a reason for hiding this comment

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

There can only be a second pass if either

 null         Allow null links                                  1 (On)
 panic        Use of "panic mode"                               1 (On)

If both are off, then there's no second pass, right?

Copy link
Member

Choose a reason for hiding this comment

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

Yes. But this is a link-parser convention only. The library doesn't know about it. For example, if the library gets a sentence_parse() request with both min_null_count==0 and max_null_count==0, it doesn't know whether a subsequent call with, e.g., min_null_count==1 and max_null_count=sent_length will follow. So maybe we can add a free_memory or even a batch_mode parse option.

Copy link
Member Author

@linas linas Jun 3, 2024

Choose a reason for hiding this comment

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

Ah, yes. I forgot what this was about. Perhaps a new struct Parse_Options_s of bool single_pass ?

// If we somehow knew that we will not be called a second
// time, then we could safely delete the Exp_pool and the
// X_node_pool, because these are no longer needed to finish
// the parse. However, if we are called a second or third time,
// during panic parsing, or with non-zero null_count, the
// disjuncts will get rebuilt, again, and the expressions are
// needed for that. So we cannot actually free this memory.
if (0 == opts->max_null_count)
{
for (WordIdx w = 0; w < sent->length; w++)
sent->word[w].x = NULL;

pool_delete(sent->Exp_pool);
sent->Exp_pool = NULL;
pool_delete(sent->X_node_pool);
sent->X_node_pool = NULL;
}
#endif

if (verbosity_level(D_PREP))
{
prt_error("Debug: After expanding expressions into disjuncts:\n\\");
print_disjunct_counts(sent);
}
print_time(opts, "Built disjuncts");

for (i=0; i<sent->length; i++)
for (WordIdx i=0; i<sent->length; i++)
{
sent->word[i].d = eliminate_duplicate_disjuncts(sent->word[i].d, false);
if (IS_GENERATION(sent->dict))
Expand Down