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.

Sign up for GitHub

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

Jump to bottom

Too slow while loading several EAMT entries from JSON configuration #363

Closed
raj-R-Reddy opened this issue Jun 3, 2021 · 6 comments
Closed

Too slow while loading several EAMT entries from JSON configuration #363

raj-R-Reddy opened this issue Jun 3, 2021 · 6 comments
Labels
Status: Tested Needs release
Milestone
4.1.6

Comments

@raj-R-Reddy
Copy link

Hi,

I have been experimenting jool_siit on CentOS-8 and figured that it is taking too long to load EAMT entries from json file. I have prepared a list of about 2000 entries (with ipv4/32 <=> ipv6/128) and it takes about 3 minutes. Is there a way to speed it up? I did try the force option but it only saves a few seconds.

Well, the issue isn't confined to json but even while inserting those one-by-one on the command line with a loop also take same amount of time.

I am looking for the persistence of EAMT entries in between the crashes/reboots but reloading them at startup seems slow.

Thanks,
Raj
ydahhrk added a commit that referenced this issue Jun 4, 2021
@ydahhrk
Radix Trie: Purge RCU waits
3a0da31 
Temporal commit; recklessly nullifies locking in the EAMT write
operations. Might yield a performance boost for #363.

Do not execute eamt add, eamt remove nor eamt display while
running this commit.
@ydahhrk
Copy link
Member

ydahhrk commented Jun 4, 2021 •
edited
Loading

Can't reproduce. I tried with 1024, 2048 and 16384 entry tables, but couldn't reach a fifth of a second with any of them. My processor is a five year old i5.

Try this commit.

WARNING: DO NOT EXECUTE jool_siit eamt add, jool_siit eamt remove NOR jool_siit eamt flush WHEN RUNNING THIS COMMIT. ALWAYS MODIFY THE TABLE THROUGH jool_siit file handle. OTHERWISE YOU WILL TEMPT THE WRATH OF CONCURRENCE.

Does that fix it?

@raj-R-Reddy
Copy link
Author

Awesome!! works like a charm.

Thank you
Raj

@ydahhrk
Copy link
Member

ydahhrk commented Jun 4, 2021 •
edited
Loading

Wait. Are you serious? Did you try running the old Jool again, in case something else might have been killing your CPU before?

Hmm. If the RCU synchronizations are really the cause of that kind of mayhem, then denylist4 can be optimized too. Never mind; denylist4 is very different.

I'm going to reopen this bug because the current solution is a half-assed hack. eamt add, remove and flush need to work properly.

@ydahhrk ydahhrk reopened this Jun 4, 2021
ydahhrk added a commit that referenced this issue Jun 4, 2021
@ydahhrk
EAMT: Fix #363 properly
2323a13 
Eliminates RCU synchronizations, but only on atomic configuration
(`file handle`) mode.

When adding EAMT entries through atomic configuration, the translator
is inactive. Therefore, there are no readers, and therefore, the RCU
syncs are redundant. So they can be thrown away at no cost.

`eamt add`, `eamt remove` and `eamt flush` were broken in the previous
commit, and now they're back in working order. (But those haven't been
optimized.)

- This optimization cannot be applied to `eamt add`, because `eamt add`
  operates on running translators.
- `eamt remove` and `eamt flush` never needed optimizations. Turns out
  I broke them for no reason.

I don't really know why synchronize_rcu() is so slow in some systems,
but presumably, it is implemented differently depending on architecture.
@ydahhrk
Copy link
Member

ydahhrk commented Jun 4, 2021

@raj-R-Reddy: I just uploaded a proper fix for the bug.

It would be helpful if you would test it, and report whether it works for you.

@ydahhrk ydahhrk added the Status: Coded Needs testing and release label Jun 5, 2021
@raj-R-Reddy
Copy link
Author

@ydahhrk: I have tested with 3000 entries and it loads in 0.3 seconds on a xeon 2.5GHz CPU.

@ydahhrk
Copy link
Member

ydahhrk commented Jun 5, 2021

Ok, thank you.

@ydahhrk ydahhrk closed this as completed Jun 5, 2021
@ydahhrk ydahhrk added Status: Tested Needs release and removed Status: Coded Needs testing and release labels Jun 5, 2021
@ydahhrk ydahhrk added this to the 4.1.6 milestone Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Status: Tested Needs release
Projects
None yet
Milestone
4.1.6
Development

No branches or pull requests
2 participants
@ydahhrk @raj-R-Reddy