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

Fast field access #97784

Closed
wants to merge 12 commits into from
Closed

Fast field access #97784

wants to merge 12 commits into from

Conversation

steveharter
Copy link
Member

@steveharter steveharter commented Jan 31, 2024

[verifying tests]

Click for benchmarks
| Method                 | Job        | Toolchain                 | Mean      | Error     | StdDev    | Median    | Min       | Max       | Ratio | RatioSD | Gen0   | Allocated | Alloc Ratio |
|----------------------- |----------- |-------------------------- |----------:|----------:|----------:|----------:|----------:|----------:|------:|--------:|-------:|----------:|------------:|
| Field_Get_int          | Job-GWENOV | \FIELD_AFTER\corerun.exe  | 30.356 ns | 0.2293 ns | 0.2145 ns | 30.280 ns | 30.074 ns | 30.744 ns |  1.00 |    0.00 | 0.0022 |      24 B |        1.00 |
| Field_Get_int          | Job-IVMTYD | \FIELD_BEFORE\corerun.exe | 36.561 ns | 0.3267 ns | 0.3056 ns | 36.582 ns | 35.810 ns | 37.027 ns |  1.20 |    0.01 | 0.0022 |      24 B |        1.00 |
|                        |            |                           |           |           |           |           |           |           |       |         |        |           |             |
| Field_GetStatic_int    | Job-GWENOV | \FIELD_AFTER\corerun.exe  | 28.014 ns | 0.3946 ns | 0.3295 ns | 27.926 ns | 27.604 ns | 28.618 ns |  1.00 |    0.00 | 0.0023 |      24 B |        1.00 |
| Field_GetStatic_int    | Job-IVMTYD | \FIELD_BEFORE\corerun.exe | 37.240 ns | 0.2787 ns | 0.2607 ns | 37.181 ns | 36.978 ns | 37.791 ns |  1.33 |    0.02 | 0.0023 |      24 B |        1.00 |
|                        |            |                           |           |           |           |           |           |           |       |         |        |           |             |
| Field_Get_class        | Job-GWENOV | \FIELD_AFTER\corerun.exe  |  3.986 ns | 0.0228 ns | 0.0191 ns |  3.990 ns |  3.948 ns |  4.023 ns |  1.00 |    0.00 |      - |         - |          NA |
| Field_Get_class        | Job-IVMTYD | \FIELD_BEFORE\corerun.exe | 23.594 ns | 0.1207 ns | 0.1008 ns | 23.582 ns | 23.437 ns | 23.781 ns |  5.92 |    0.02 |      - |         - |          NA |
|                        |            |                           |           |           |           |           |           |           |       |         |        |           |             |
| Field_GetStatic_class  | Job-GWENOV | \FIELD_AFTER\corerun.exe  |  2.660 ns | 0.0229 ns | 0.0203 ns |  2.656 ns |  2.630 ns |  2.707 ns |  1.00 |    0.00 |      - |         - |          NA |
| Field_GetStatic_class  | Job-IVMTYD | \FIELD_BEFORE\corerun.exe | 19.832 ns | 0.1460 ns | 0.1366 ns | 19.814 ns | 19.615 ns | 20.061 ns |  7.46 |    0.10 |      - |         - |          NA |
|                        |            |                           |           |           |           |           |           |           |       |         |        |           |             |
| Field_Get_struct       | Job-GWENOV | \FIELD_AFTER\corerun.exe  | 34.247 ns | 0.3703 ns | 0.3283 ns | 34.381 ns | 33.481 ns | 34.583 ns |  1.00 |    0.00 | 0.0022 |      24 B |        1.00 |
| Field_Get_struct       | Job-IVMTYD | \FIELD_BEFORE\corerun.exe | 44.022 ns | 0.2079 ns | 0.1843 ns | 43.980 ns | 43.676 ns | 44.404 ns |  1.29 |    0.01 | 0.0023 |      24 B |        1.00 |
|                        |            |                           |           |           |           |           |           |           |       |         |        |           |             |
| Field_GetStatic_struct | Job-GWENOV | \FIELD_AFTER\corerun.exe  | 30.300 ns | 0.2982 ns | 0.2328 ns | 30.358 ns | 29.921 ns | 30.592 ns |  1.00 |    0.00 | 0.0023 |      24 B |        1.00 |
| Field_GetStatic_struct | Job-IVMTYD | \FIELD_BEFORE\corerun.exe | 48.208 ns | 0.2261 ns | 0.1888 ns | 48.173 ns | 47.968 ns | 48.709 ns |  1.59 |    0.01 | 0.0022 |      24 B |        1.00 |
|                        |            |                           |           |           |           |           |           |           |       |         |        |           |             |
| Field_Set_int          | Job-GWENOV | \FIELD_AFTER\corerun.exe  |  7.820 ns | 0.0474 ns | 0.0370 ns |  7.831 ns |  7.739 ns |  7.863 ns |  1.00 |    0.00 | 0.0023 |      24 B |        1.00 |
| Field_Set_int          | Job-IVMTYD | \FIELD_BEFORE\corerun.exe | 33.892 ns | 0.2985 ns | 0.2792 ns | 33.932 ns | 33.405 ns | 34.520 ns |  4.33 |    0.03 | 0.0023 |      24 B |        1.00 |
|                        |            |                           |           |           |           |           |           |           |       |         |        |           |             |
| Field_SetStatic_int    | Job-GWENOV | \FIELD_AFTER\corerun.exe  |  6.887 ns | 0.0517 ns | 0.0458 ns |  6.888 ns |  6.818 ns |  6.988 ns |  1.00 |    0.00 | 0.0023 |      24 B |        1.00 |
| Field_SetStatic_int    | Job-IVMTYD | \FIELD_BEFORE\corerun.exe | 39.479 ns | 0.7245 ns | 0.6777 ns | 39.163 ns | 38.879 ns | 40.939 ns |  5.74 |    0.11 | 0.0022 |      24 B |        1.00 |
|                        |            |                           |           |           |           |           |           |           |       |         |        |           |             |
| Field_Set_class        | Job-GWENOV | \FIELD_AFTER\corerun.exe  | 15.787 ns | 0.1478 ns | 0.1310 ns | 15.736 ns | 15.655 ns | 16.045 ns |  1.00 |    0.00 | 0.0023 |      24 B |        1.00 |
| Field_Set_class        | Job-IVMTYD | \FIELD_BEFORE\corerun.exe | 35.001 ns | 0.3132 ns | 0.2930 ns | 35.019 ns | 34.417 ns | 35.645 ns |  2.22 |    0.02 | 0.0023 |      24 B |        1.00 |
|                        |            |                           |           |           |           |           |           |           |       |         |        |           |             |
| Field_SetStatic_class  | Job-GWENOV | \FIELD_AFTER\corerun.exe  | 14.618 ns | 0.1222 ns | 0.1084 ns | 14.610 ns | 14.441 ns | 14.814 ns |  1.00 |    0.00 | 0.0023 |      24 B |        1.00 |
| Field_SetStatic_class  | Job-IVMTYD | \FIELD_BEFORE\corerun.exe | 44.213 ns | 0.3276 ns | 0.2904 ns | 44.146 ns | 43.807 ns | 44.734 ns |  3.02 |    0.02 | 0.0023 |      24 B |        1.00 |
|                        |            |                           |           |           |           |           |           |           |       |         |        |           |             |
| Field_Set_struct       | Job-GWENOV | \FIELD_AFTER\corerun.exe  | 29.993 ns | 0.3763 ns | 0.3520 ns | 29.869 ns | 29.356 ns | 30.761 ns |  1.00 |    0.00 | 0.0022 |      24 B |        1.00 |
| Field_Set_struct       | Job-IVMTYD | \FIELD_BEFORE\corerun.exe | 29.903 ns | 0.1218 ns | 0.1080 ns | 29.943 ns | 29.697 ns | 30.032 ns |  1.00 |    0.01 | 0.0022 |      24 B |        1.00 |
|                        |            |                           |           |           |           |           |           |           |       |         |        |           |             |
| Field_SetStatic_struct | Job-GWENOV | \FIELD_AFTER\corerun.exe  | 38.465 ns | 0.5719 ns | 0.5069 ns | 38.359 ns | 37.741 ns | 39.556 ns |  1.00 |    0.00 | 0.0022 |      24 B |        1.00 |
| Field_SetStatic_struct | Job-IVMTYD | \FIELD_BEFORE\corerun.exe | 37.598 ns | 0.4758 ns | 0.4451 ns | 37.635 ns | 36.262 ns | 38.099 ns |  0.98 |    0.02 | 0.0022 |      24 B |        1.00 |

@ghost
Copy link

ghost commented Jan 31, 2024

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: steveharter
Assignees: steveharter
Labels:

area-System.Reflection

Milestone: -

GCPROTECT_BEGIN(refField);

if (refField->GetField()->IsThreadStatic()) {
ret = Thread::GetStaticFieldAddress(pFieldDesc);
Copy link
Member

Choose a reason for hiding this comment

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

These addresses may be in GC allocated memory in some cases. They can move and need to be reported to the GC.


// IsFastPathSupported needs to checked before calling this method.
_ASSERTE(!pFieldDesc->IsEnCNew());
_ASSERTE(!refField->GetField()->IsThreadStatic());
Copy link
Member

Choose a reason for hiding this comment

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

Addresses of regular static fields in collectitble assemblies are allocated on the GC heap and can move.

This needs to have similar set of cases as the JIT:

if (pFieldMT->Collectible())
{
// Static fields are not pinned in collectible types. We will always access
// them using a helper since the address cannot be embedded into the code.
fieldAccessor = CORINFO_FIELD_STATIC_SHARED_STATIC_HELPER;
pResult->helper = getSharedStaticsHelper(pField, pFieldMT);
}
else if (pField->IsThreadStatic())

}
else
{
_addressOrOffset = RuntimeFieldHandle.GetInstanceFieldOffset(_fieldInfo);
Copy link
Contributor

Choose a reason for hiding this comment

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

AFAIK fields added in EnC don't have a valid offset that's inside of the object.

@steveharter
Copy link
Member Author

steveharter commented Feb 8, 2024

Closing due to corrupt branch; will reopen new PR; see #98199

@steveharter steveharter closed this Feb 8, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants