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

Update snmalloc from 0.5.0 to 0.5.2 #2114

Closed
wants to merge 2 commits into from

Conversation

achamayou
Copy link
Member

No description provided.

@achamayou achamayou requested a review from a team as a code owner January 26, 2021 10:06
@achamayou achamayou mentioned this pull request Jan 26, 2021
4 tasks
@ghost
Copy link

ghost commented Jan 26, 2021

snmalloc_0.5.2@18062 aka 20210127.2 vs master ewma over 50 builds from 17400 to 18060
images

@achamayou
Copy link
Member Author

Interestingly, this upgrade is causing the following tests to fail reproducibly:

The following tests FAILED:
	 46 - full_test_suite (Failed)
	 69 - forum_app (Failed)

@achamayou
Copy link
Member Author

There is a major performance regression in virtual mode, which is causing transaction times to double in the forum_app for example, and to timeout in others.

Run with snmalloc 0.5.0:

69: (Use `node --trace-warnings ...` to show where the warning was created)
69:         ✓ creates numeric polls (2817ms)
69:         ✓ creates string polls (2877ms)
69:         ✓ rejects creating polls with an existing topic (5591ms)
69:         ✓ rejects creating polls without authorization (2500ms)
69:       POST /
69:         ✓ creates multiple polls (2673ms)
69:         ✓ rejects creating polls with an existing topic (5406ms)
69:         ✓ rejects creating polls without authorization (2457ms)
69:       PUT /{topic}
69:         ✓ stores opinions to a topic (5572ms)
69:         ✓ rejects opinions with mismatching data type (5555ms)
69:         ✓ rejects opinions for unknown topics (2771ms)
69:         ✓ rejects opinions without authorization (5167ms)
69:       PUT /
69:         ✓ stores opinions to multiple topics (5465ms)
69:         ✓ rejects opinions with mismatching data type (5518ms)
69:         ✓ rejects opinions for unknown topics (2694ms)
69:         ✓ rejects opinions without authorization (5183ms)
69:       GET /{topic}
69:         ✓ returns aggregated numeric poll opinions (33325ms)
69:         ✓ returns aggregated string poll opinions (33694ms)
69:         ✓ rejects returning aggregated opinions below the required opinion count threshold (30365ms)
69:         ✓ rejects returning aggregated opinions for unknown topics (2766ms)
69:     /csv
69:       GET|POST /
69:         ✓ stores and returns opinions of authenticated user as CSV (4650ms)

0.5.2

74: (Use `node --trace-warnings ...` to show where the warning was created)
74:         ✓ creates numeric polls (6815ms)
74:         ✓ creates string polls (6788ms)
74:         ✓ rejects creating polls with an existing topic (13528ms)
74:         ✓ rejects creating polls without authorization (6455ms)
74:       POST /
74:         ✓ creates multiple polls (6653ms)
74:         ✓ rejects creating polls with an existing topic (13440ms)
74:         ✓ rejects creating polls without authorization (6425ms)
74:       PUT /{topic}
74:         ✓ stores opinions to a topic (13401ms)
74:         ✓ rejects opinions with mismatching data type (13508ms)
74:         ✓ rejects opinions for unknown topics (6666ms)
74:         ✓ rejects opinions without authorization (13211ms)
74:       PUT /
74:         ✓ stores opinions to multiple topics (13355ms)
74:         ✓ rejects opinions with mismatching data type (13420ms)
74:         ✓ rejects opinions for unknown topics (6683ms)
74:         ✓ rejects opinions without authorization (13080ms)
74:       GET /{topic}
74:         1) returns aggregated numeric poll opinions
74:         2) returns aggregated string poll opinions
74:         3) rejects returning aggregated opinions below the required opinion count threshold
74:         ✓ rejects returning aggregated opinions for unknown topics (21441ms)
74:     /csv
74:       GET|POST /
74:         4) stores and returns opinions of authenticated user as CSV
74: [15:24:38.849] Stopping all CCF nodes...
74: 
74: [15:24:43.685] All CCF nodes stopped.

@achamayou
Copy link
Member Author

Ok so:

  1. This goes away if LVI_MITIGATIONS are turned off
  2. This is with everything (including snmalloc) built as Debug

@achamayou
Copy link
Member Author

Bisected down to microsoft/snmalloc@d3ecd66

@achamayou
Copy link
Member Author

@mjp41 kindly made a tweak to the pow2 check code used by the assertion called in align_up/down (microsoft/snmalloc#274), which does resolve the performance hit. Closing this for now, will re-open a 0.5.0 to 0.5.3 PR once available.

@achamayou achamayou closed this Jan 27, 2021
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