Dependency linearization clean up #41

New Issue

deavmi · 2022-12-13T18:02:48Z

deavmi commented

2022-12-13 18:02:48 +00:00

Calling print() will linearize and print a tree whilst doing so, returned as a string from this function. However, it adds to a static field the linearized (DFS searched items), this is the static DNode[] poes list.

Problem 1

Calling tree() again on the same DNode would technically double this list.

Fix: It should not do this, maybe have this stored or cached, to prevent this from happening

Problem 2

Fix: Somehow de-couple tree and lineraization or cache the stuff, I think that may make the most sense.

Problem 3

Fix: We should not be access via public static DNode[] poes but rather via a getLinearized().

Problem 4 (not a problem)

This is not really a problem but may be nice.

Clear all isVisited before lineairizng, and use that instead of isCompleted
Will also allow re-linearization (if we want that even though I don't see why)

Problem 5

Why do we have, isDone() again?

if(isDone)
{
	destinationLinearList ~= this;
}

And is defined as:

private bool isDone()
{
        bool done = false;

        foreach(DNode dependency; dependencies)
        {
            if(!dependency.isCompleted())
            {
                return false;
            }
        }

        return true;
}

Mmmh.... Wait, maybe it decycled things? No surely that is what the if(!dependancy.isCompleted()) in the for-loop does.

Problem 6

Throw DependencyException when there are any misuses of these functions

Calling `print()` will linearize and print a tree whilst doing so, returned as a string from this function. However, it adds to a static field the linearized (DFS searched items), this is the `static DNode[] poes` list. ## Problem 1 Calling `tree()` again on the **same** DNode would technically double this list. - [x] **Fix**: It should not do this, maybe have this stored or cached, to prevent this from happening ## Problem 2 - [x] **Fix**: Somehow de-couple tree and lineraization or cache the stuff, I think that may make the most sense. ## Problem 3 - [x] **Fix**: We should not be access via `public static DNode[] poes` but rather via a `getLinearized()`. ## Problem 4 (not a problem) This is not really a problem but may be nice. - [ ] Clear all `isVisited` before lineairizng, and use that instead of `isCompleted` - [ ] Will also allow re-linearization (if we want that even though I don't see why) ## Problem 5 Why do we have, `isDone()` again? ```d if(isDone) { destinationLinearList ~= this; } ``` And is defined as: ```d private bool isDone() { bool done = false; foreach(DNode dependency; dependencies) { if(!dependency.isCompleted()) { return false; } } return true; } ``` Mmmh.... Wait, maybe it decycled things? No surely that is what the `if(!dependancy.isCompleted())` in the for-loop does. ## Problem 6 - [x] Throw `DependencyException` when there are any misuses of these functions

deavmi commented

2022-12-13 18:05:01 +00:00

I think, for Problem 2, we should have a linearize() that then caches (so oneshot execute).

Whilst it does this it generates tree and caches, therefore calling (Problem 1) tree() will return cached tree generation.

I think, for Problem 2, we should have a `linearize()` that then caches (so oneshot execute). Whilst it does this it generates tree and caches, therefore calling (Problem 1) `tree()` will return cached tree generation.

deavmi added reference vardec_varass_dependency

2022-12-13 18:05:07 +00:00

deavmi added the

dependency

label 2022-12-13 18:05:13 +00:00

deavmi added this to the Dependency tree, type-checking and codegen project 2022-12-13 18:05:17 +00:00

deavmi self-assigned this 2022-12-13 18:05:20 +00:00

deavmi added a new dependency 2022-12-14 09:30:51 +00:00

#45 Dependency

deavmi added the due date 2023-01-15

2023-01-20 13:15:22 +00:00

deavmi modified the due date from 2023-01-15 to 2023-01-22

2023-01-20 13:15:28 +00:00

deavmi commented

2023-01-20 13:15:35 +00:00

I'm going to look at this this afternoon.

deavmi commented

2023-01-20 13:16:07 +00:00

Made a new branch for this linearizer_cleanup

Made a new branch for this `linearizer_cleanup`

deavmi changed reference from ~~vardec_varass_dependency~~ to linearizer_cleanup

2023-01-20 13:17:13 +00:00

deavmi commented

2023-01-20 14:53:19 +00:00

Solutions (so far) ✅

Problem 1

This has been sorted out with a hasLinearized check, so caching is enabled. As of commit 68e1a25c9a162f4cf6b25278db7bc2cb88f76996 on the branch of this issue.

Problem 2

This has been sorted out with our performLinearization(), getLinearizedNodes() and getTree() methods. As of commit 68e1a25c9a162f4cf6b25278db7bc2cb88f76996 on the branch of this issue.

Problem 3

We have removed all references to static DNode[] poes in the DNode class. As of commit 68e1a25c9a162f4cf6b25278db7bc2cb88f76996 on the branch of this issue.

## Solutions (so far) ✅ ### Problem 1 This has been sorted out with a `hasLinearized` check, so caching is enabled. As of commit `68e1a25c9a162f4cf6b25278db7bc2cb88f76996` on the branch of this issue. ### Problem 2 This has been sorted out with our `performLinearization()`, `getLinearizedNodes()` and `getTree()` methods. As of commit `68e1a25c9a162f4cf6b25278db7bc2cb88f76996` on the branch of this issue. ### Problem 3 We have removed all references to `static DNode[] poes` in the `DNode` class. As of commit `68e1a25c9a162f4cf6b25278db7bc2cb88f76996` on the branch of this issue.

deavmi added this to the Basics milestone 2023-01-20 14:54:13 +00:00

deavmi modified the due date from 2023-01-22 to 2023-01-21

2023-01-20 14:55:13 +00:00

deavmi started working 2023-01-20 14:55:16 +00:00

deavmi commented

2023-01-20 14:58:08 +00:00

Merged the fixes for Problem 1, 2 and 3 to varass_vardec_dependency ✅

Merged the fixes for Problem 1, 2 and 3 to `varass_vardec_dependency` ✅

deavmi commented

2023-01-20 18:16:08 +00:00

Problem 6 has been solved with commit 57ebd443b0f0517ad474d004d6e31ae01a3eff02 ov the branch of this issue.

Problem 6 has been solved with commit `57ebd443b0f0517ad474d004d6e31ae01a3eff02` ov the branch of this issue.

deavmi commented

2023-01-20 18:16:44 +00:00

Merged the fixes for Problem 6 to varass_vardec_dependency ✅

Merged the fixes for Problem 6 to `varass_vardec_dependency` ✅

deavmi removed the due date 2023-01-21

2023-01-20 20:17:36 +00:00

deavmi added the due date 2023-02-15

2023-01-20 20:17:50 +00:00

deavmi stopped working 2023-01-20 20:17:52 +00:00

5 hours 22 minutes

deavmi commented

2023-01-22 12:35:06 +00:00

Thinking 🤔

I think nodePool must remain static as this is how several DNodeGenerator instances pool a variable processed in the module pass (with a seperate DNodeGenerator) and know it has been visited (i.e. declared and all), because this static DNode[] nodePool is same place in memory across instances of DNodeGenerator as it is static.

There isn't anything bad about this, we could - for memory leaking reasons - clear it after each compilation however.

## Thinking 🤔 I think `nodePool` must remain `static` as this is how several `DNodeGenerator` instances pool a variable processed in the module pass (with a seperate DNodeGenerator) and know it has been visited (i.e. declared and all), because this `static DNode[] nodePool` is same place in memory across instances of `DNodeGenerator` as it is `static`. There isn't anything bad about this, we could - for memory leaking reasons - clear it after each compilation however.

deavmi commented

2023-12-06 09:09:19 +00:00

Thinking 🤔

I think nodePool must remain static as this is how several DNodeGenerator instances pool a variable processed in the module pass (with a seperate DNodeGenerator) and know it has been visited (i.e. declared and all), because this static DNode[] nodePool is same place in memory across instances of DNodeGenerator as it is static.

There isn't anything bad about this, we could - for memory leaking reasons - clear it after each compilation however.

I find this retarded, perhaps each DNodeGenerator must be provided with some PoolManager or something, and passed in. That is the neatest to me, in my opinion.

See #161

> ## Thinking 🤔 > > I think `nodePool` must remain `static` as this is how several `DNodeGenerator` instances pool a variable processed in the module pass (with a seperate DNodeGenerator) and know it has been visited (i.e. declared and all), because this `static DNode[] nodePool` is same place in memory across instances of `DNodeGenerator` as it is `static`. > > There isn't anything bad about this, we could - for memory leaking reasons - clear it after each compilation however. I find this retarded, perhaps each `DNodeGenerator` must be provided with some `PoolManager` or something, and passed in. That is the neatest to me, in my opinion. See #161

deavmi added a new dependency 2023-12-06 19:06:53 +00:00

#161 🧹️🧼 Cleanup: Pool provider

deavmi commented

2023-12-09 20:43:48 +00:00

Also #162

deavmi modified the due date from 2023-02-15 to 2023-12-31

2023-12-10 10:23:35 +00:00

deavmi started working 2024-05-07 10:49:45 +01:00

deavmi commented

2024-05-07 10:50:06 +01:00

Is this done or not?