Skip to content

Conversation

Keno
Copy link
Member

@Keno Keno commented Jun 9, 2025

When this API was added, this function inlined, which is important, because the API relies on the allocation of the Ref being elided. At some point (I went back to 1.8) this regressed. For example, it is currently responsible for substantially all non-Expr allocations in JuliaParser. Before (parsing all of Base with JuliaParser):

│     Memory estimate: 76.93 MiB, allocs estimate: 719922.

After:

│     Memory estimate: 53.31 MiB, allocs estimate: 156.

Also add a test to make sure this doesn't regress again.

@Keno Keno added backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 backport 1.12 Change should be backported to release-1.12 labels Jun 9, 2025
Keno added a commit to JuliaLang/JuliaSyntax.jl that referenced this pull request Jun 9, 2025
@Seelengrab
Copy link
Contributor

Do we know why this stopped being inlined? Is it because of the effects tainting on the ccall?

@Keno
Copy link
Member Author

Keno commented Jun 9, 2025

We must have changed the ccall cost model at some point. Usually it's not a good idea to inline, so that's sensible, it's just that it's really required in this case.

@giordano giordano added the unicode Related to unicode characters and encodings label Jun 9, 2025
When this API was added, this function inlined, which is important,
because the API relies on the allocation of the `Ref` being elided.
At some point (I went back to 1.8) this regressed. For example, it
is currently responsible for substantially all non-Expr allocations
in JuliaParser. Before (parsing all of Base with JuliaParser):
```
│     Memory estimate: 76.93 MiB, allocs estimate: 719922.
```
After:
```
│     Memory estimate: 53.31 MiB, allocs estimate: 156.
```

Also add a test to make sure this doesn't regress again.
@Keno Keno force-pushed the kf/inlineisgraphemebreak branch from b207111 to 4af681c Compare June 9, 2025 08:57
@JeffBezanson JeffBezanson added performance Must go faster parser Language parsing and surface syntax labels Jun 9, 2025
@Keno Keno changed the title Unicdode: Force-inline isgraphemebreak! Unicode: Force-inline isgraphemebreak! Jun 10, 2025
@Keno Keno merged commit d6294ba into master Jun 10, 2025
9 of 10 checks passed
@Keno Keno deleted the kf/inlineisgraphemebreak branch June 10, 2025 04:27
KristofferC pushed a commit that referenced this pull request Jun 11, 2025
When this API was added, this function inlined, which is important,
because the API relies on the allocation of the `Ref` being elided. At
some point (I went back to 1.8) this regressed. For example, it is
currently responsible for substantially all non-Expr allocations in
JuliaParser. Before (parsing all of Base with JuliaParser):
```
│     Memory estimate: 76.93 MiB, allocs estimate: 719922.
```
After:
```
│     Memory estimate: 53.31 MiB, allocs estimate: 156.
```

Also add a test to make sure this doesn't regress again.

(cherry picked from commit d6294ba)
Keno added a commit to JuliaLang/JuliaSyntax.jl that referenced this pull request Jun 12, 2025
nilesh646 pushed a commit to nilesh646/julia that referenced this pull request Jun 17, 2025
When this API was added, this function inlined, which is important,
because the API relies on the allocation of the `Ref` being elided. At
some point (I went back to 1.8) this regressed. For example, it is
currently responsible for substantially all non-Expr allocations in
JuliaParser. Before (parsing all of Base with JuliaParser):
```
│     Memory estimate: 76.93 MiB, allocs estimate: 719922.
```
After:
```
│     Memory estimate: 53.31 MiB, allocs estimate: 156.
```

Also add a test to make sure this doesn't regress again.
@KristofferC KristofferC mentioned this pull request Jun 25, 2025
60 tasks
KristofferC pushed a commit that referenced this pull request Jun 25, 2025
When this API was added, this function inlined, which is important,
because the API relies on the allocation of the `Ref` being elided. At
some point (I went back to 1.8) this regressed. For example, it is
currently responsible for substantially all non-Expr allocations in
JuliaParser. Before (parsing all of Base with JuliaParser):
```
│     Memory estimate: 76.93 MiB, allocs estimate: 719922.
```
After:
```
│     Memory estimate: 53.31 MiB, allocs estimate: 156.
```

Also add a test to make sure this doesn't regress again.

(cherry picked from commit d6294ba)
@KristofferC KristofferC mentioned this pull request Jun 25, 2025
71 tasks
KristofferC pushed a commit that referenced this pull request Jul 3, 2025
When this API was added, this function inlined, which is important,
because the API relies on the allocation of the `Ref` being elided. At
some point (I went back to 1.8) this regressed. For example, it is
currently responsible for substantially all non-Expr allocations in
JuliaParser. Before (parsing all of Base with JuliaParser):
```
│     Memory estimate: 76.93 MiB, allocs estimate: 719922.
```
After:
```
│     Memory estimate: 53.31 MiB, allocs estimate: 156.
```

Also add a test to make sure this doesn't regress again.

(cherry picked from commit d6294ba)
@KristofferC KristofferC removed the backport 1.12 Change should be backported to release-1.12 label Jul 9, 2025
KristofferC pushed a commit that referenced this pull request Aug 19, 2025
When this API was added, this function inlined, which is important,
because the API relies on the allocation of the `Ref` being elided. At
some point (I went back to 1.8) this regressed. For example, it is
currently responsible for substantially all non-Expr allocations in
JuliaParser. Before (parsing all of Base with JuliaParser):
```
│     Memory estimate: 76.93 MiB, allocs estimate: 719922.
```
After:
```
│     Memory estimate: 53.31 MiB, allocs estimate: 156.
```

Also add a test to make sure this doesn't regress again.

(cherry picked from commit d6294ba)
KristofferC pushed a commit that referenced this pull request Aug 19, 2025
When this API was added, this function inlined, which is important,
because the API relies on the allocation of the `Ref` being elided. At
some point (I went back to 1.8) this regressed. For example, it is
currently responsible for substantially all non-Expr allocations in
JuliaParser. Before (parsing all of Base with JuliaParser):
```
│     Memory estimate: 76.93 MiB, allocs estimate: 719922.
```
After:
```
│     Memory estimate: 53.31 MiB, allocs estimate: 156.
```

Also add a test to make sure this doesn't regress again.

(cherry picked from commit d6294ba)
@KristofferC KristofferC removed the backport 1.11 Change should be backported to release-1.11 label Aug 19, 2025
KristofferC pushed a commit that referenced this pull request Aug 19, 2025
When this API was added, this function inlined, which is important,
because the API relies on the allocation of the `Ref` being elided. At
some point (I went back to 1.8) this regressed. For example, it is
currently responsible for substantially all non-Expr allocations in
JuliaParser. Before (parsing all of Base with JuliaParser):
```
│     Memory estimate: 76.93 MiB, allocs estimate: 719922.
```
After:
```
│     Memory estimate: 53.31 MiB, allocs estimate: 156.
```

Also add a test to make sure this doesn't regress again.

(cherry picked from commit d6294ba)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.10 Change should be backported to the 1.10 release parser Language parsing and surface syntax performance Must go faster unicode Related to unicode characters and encodings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants